fix(security): clear all open CodeQL code-scanning alerts#158
Merged
Conversation
Triage + fixes for the 32 open alerts under the Security tab: Real fixes: - py/log-injection (17): every live site now scrubs the user-controlled value (character/guild names, search queries, cache keys, audit actor) via backend.core.log_safety.scrub before it reaches a log line — views, characters, claim, guild, guild_cache, cache, aa, parses ingest, audit_log. Several flagged sites had already been deleted by the catalogue rearchitecture; their live equivalents are covered. - py/xml-bomb: the ACT paste-import now parses with defusedxml (new dependency) so entity-expansion bombs are rejected, not expanded; DefusedXmlException joins ParseError in the 400 handler. Verified a billion-laughs payload returns 400. - py/incomplete-url-substring-sanitization: test_auth asserts urlparse(location).hostname == "discord.com" instead of a substring. Dismissed as false positives (documented on each alert): - py/weak-sensitive-data-hashing: SHA-256 over a 192-bit random bearer token, not a password. - py/clear-text-logging-sensitive-data (9): the logged values are token_id (integer row id), display prefix, user_id, access_status, remote IP — operational identifiers; raw tokens/signatures are never logged per CLAUDE.md's sensitive-values rule. CodeQL taints entire rows/dicts derived from a token. - py/path-injection app.py: the SPA fallback resolves + containment- checks (relative_to) right below the flagged line. Staleness fix: the CodeQL workflow gains workflow_dispatch so the main-branch alert set can be refreshed on demand (it deliberately has no push trigger — Railway's deploy gate waits on push workflows; the weekly cron was the only main refresh, leaving alerts pointing at code deleted weeks ago, e.g. the aa.py path-injection pair from the pre-#141 tree-JSON reads). 1527 tests green, ruff + pyright clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
VortexUK
added a commit
that referenced
this pull request
Jul 11, 2026
The refreshed main CodeQL analysis (post-#158) left three live alerts: the cache HIT log (one key log site missed in #158) and the aa.py effects-parse warning (crc/tier are FastAPI-typed ints that cannot carry CRLF, but scrub is free and clears the taint). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Triage of the 32 open alerts in Security → Code scanning: real fixes for three rule classes, documented dismissals for the false-positive classes, and a workflow fix for the staleness that let dead code haunt the list.
Real fixes
log_safety.scrubbefore logging. A few flagged sites no longer exist (deleted by the catalogue rearchitecture); their live equivalents are covered.DefusedXmlExceptionjoinsParseErrorin the error handler.urlparse(location).hostname == "discord.com"rather than a substring.Dismissed with per-alert justification (11)
token_id(integer row id), display prefix,user_id,access_status, remote IP: operational identifiers, not credentials. Raw tokens/signature bytes are never logged (CLAUDE.md's sensitive-values rule); CodeQL taints whole rows derived from a token.resolve()d and containment-checked withrelative_todirectly below the flagged line; CodeQL doesn't model that barrier.Staleness
The CodeQL workflow deliberately has no push trigger (Railway's deploy gate waits on push workflows), so main's alert set only refreshed on the weekly cron — the two
aa.pypath-injection alerts point at tree-JSON-reading code deleted back in #141. Added workflow_dispatch so main analysis can be refreshed on demand without gating deploys; I'll dispatch a run after merge, which should auto-close the stale alerts as fixed.Verification
1527 tests green, ruff + pyright clean, billion-laughs payload rejected 400.
🤖 Generated with Claude Code