Skip to content

docs(security): add inline CodeQL sanitizer comments at 5 path-injection sites - #491

Closed
liususan091219 wants to merge 1 commit into
mainfrom
fix/codeql-sanitization-comments
Closed

liususan091219 wants to merge 1 commit into
mainfrom
fix/codeql-sanitization-comments

Conversation

@liususan091219

Copy link
Copy Markdown
Collaborator

Summary

Adds explicit comment markers at every sanitization layer (regex strip + is_relative_to bound check) so the won't-fix dismissal rationale for the open py/path-injection alerts is visible directly in the source.

No code logic change — comments only. Both files parse cleanly.

Sites covered

  • src/agent-api.py — _safe_path helper docstring + 2 sanitizer markers; /media/ handler 3-of-3 markers; answer-task POST handler 2-of-2 markers
  • src/dashboard.py — GET /notes/ handler 3-of-3 markers; DELETE /notes/ handler 3-of-3 markers

Each marker reads e.g.:

# CodeQL sanitizer (1/3): regex allowlist strips everything except [a-zA-Z0-9_./-]

Background

After the post-merge audit on Chi's #485-#488 closed 9 of 18 alerts, 11 path-injection alerts remained open (#17, #18, #19, #23, #28, #29, #30, #31, #45, #46, #47). Local CodeQL re-run on 2026-04-21 (codeql 2.25.2 + custom model-pack experiments) confirmed:

  1. The remaining alerts are real taint flows — not false positives
  2. Every flagged sink already has inline sanitization: regex allowlist + is_relative_to bound check
  3. CodeQL's py/path-injection query doesn't recognize Path.is_relative_to as a sanitizer
  4. Custom model-pack barrierGuardModel + barrierModel experiments didn't close the alerts (likely needs custom QL extension, not just YAML)

Per Susan's direction, dismissing these 11 alerts as won't-fix once this PR merges, with each dismissal referencing the comment-marker line. The comments make the inline sanitization explicitly visible to future readers + auditors so the dismissal isn't blind.

Test plan

  • python3 -c "import ast; ast.parse(open('src/agent-api.py').read()); ast.parse(open('src/dashboard.py').read())" passes
  • git diff --stat confirms +32/-7 (comments only)
  • No behavioral change — handler logic untouched

🤖 Generated with Claude Code

…ion sites

Adds explicit comment markers at every sanitization layer (regex strip +
is_relative_to bound check) so the won't-fix dismissal rationale is visible
in the source. No code logic change; both files parse cleanly.

Sites covered:
- src/agent-api.py: _safe_path helper, /media/ handler, answer-task POST
- src/dashboard.py: GET /notes/, DELETE /notes/

Pairs with dismissal of CodeQL alerts #17, #18, #19, #23, #28, #29, #30,
#31, #45, #46, #47 (py/path-injection) — all confirmed sanitized via
local CodeQL re-run + code review on 2026-04-21.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@liususan091219

Copy link
Copy Markdown
Collaborator Author

Superseded by the call-site fix in fix/codeql-path-injection-callsites — #491 was comments-only and CodeQL ignores comments. Actual repair switches to os.path.realpath + str.startswith (the pair CodeQL models as path-injection sanitizers). Verified locally: 11 → 0 alerts. — Lucy (Mac Studio bot)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant