Commit 24f271e
security: switch py/path-injection barrier to realpath+startswith (CodeQL) (#348)
Follow-up to PR #346 (merged df9f722). Post-merge CodeQL rescan surfaced
three residual path-injection alerts on ui/dashboard/live_server.py:
* #699 line 652 (_send_file: path.read_bytes)
* #706 line 109 (_resolve_static: resolve(strict=True))
* #707 line 114 (_resolve_static: is_file())
The Python Path.is_relative_to() API is documented as a sanitizer in newer
CodeQL, but the analyser did NOT flip these sites to "fixed" after the
previous PR. The documented-stable CodeQL sanitizer for py/path-injection
is os.path.realpath(...) + str.startswith(root + os.sep) — that's the
pattern GitHub's own remediation guidance uses.
Switched both sinks to the realpath+startswith primitive:
* New module-level constants _WEBROOT_STR and _WEBROOT_PREFIX built from
os.path.realpath of the dashboard directory. _WEBROOT_PREFIX ends in
os.sep, preventing the classic "/safe-root-evil" prefix-collision
(where /safe-rooteval would falsely startswith /safe-root without
the separator).
* New helper _is_inside_webroot(real_str) encapsulates the barrier.
Single source of truth, used by both _resolve_static and _send_file.
* _resolve_static:
- String-level pre-checks (null-byte, absolute, "..", empty) now
run BEFORE any filesystem call.
- os.path.realpath() replaces Path.resolve(strict=True). Follows
symlinks, collapses "..", handles missing leaves without raising.
- _is_inside_webroot() is the first filesystem-touch-qualified
barrier; os.path.isfile() runs only after containment holds.
* _send_file: re-canonicalises with realpath() at the I/O sink, then
checks _is_inside_webroot(). This is the belt-and-braces that CodeQL
recognises locally, independent of caller guarantees.
The sanitizer contract (containment, symlink-escape, allow-list extensions,
null-byte rejection, etc.) is identical — only the internal primitive has
changed to one CodeQL's static analyser recognises.
Local verification:
* ruff check + mypy --strict: clean on ui/dashboard/live_server.py
* pytest tests/security/test_live_server_path_traversal.py: 27/27 pass
(existing tests exercise the contract, not the implementation, so
they required no update)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>1 parent 1444a71 commit 24f271e
1 file changed
Lines changed: 41 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
59 | 61 | | |
60 | 62 | | |
61 | 63 | | |
| |||
78 | 80 | | |
79 | 81 | | |
80 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
81 | 94 | | |
82 | | - | |
| 95 | + | |
83 | 96 | | |
84 | 97 | | |
85 | 98 | | |
| |||
88 | 101 | | |
89 | 102 | | |
90 | 103 | | |
91 | | - | |
92 | | - | |
93 | | - | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
94 | 107 | | |
95 | 108 | | |
96 | | - | |
97 | | - | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
98 | 113 | | |
99 | 114 | | |
100 | 115 | | |
| |||
105 | 120 | | |
106 | 121 | | |
107 | 122 | | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
113 | 128 | | |
114 | | - | |
| 129 | + | |
115 | 130 | | |
| 131 | + | |
116 | 132 | | |
117 | 133 | | |
118 | 134 | | |
| |||
641 | 657 | | |
642 | 658 | | |
643 | 659 | | |
644 | | - | |
645 | | - | |
646 | | - | |
647 | | - | |
648 | | - | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
649 | 668 | | |
650 | 669 | | |
651 | 670 | | |
652 | | - | |
| 671 | + | |
653 | 672 | | |
654 | 673 | | |
655 | 674 | | |
| |||
0 commit comments