Repository navigation
ci(security): codeql-config.yml's file-scoped query-filters exclude nothing, under either setup #16302
Description
Activity
Owner decision, 11 Sep 2026: option 1. Dismiss each matching alert in code scanning, with the documented reason. Remove the three file-scoped
query-filtersentries fromcodeql-config.yml, or turn them into reason-only comments, so the config stops claiming exclusions it can't make. Add a guard so a file-scopedquery-filtersentry can't be added and silently do nothing again.Order: the dismissals happen after #16304's switch to advanced setup (its post-merge settings steps). Default-setup and advanced-setup alerts are tracked separately, so dismissing today's default-setup alerts wouldn't carry over. The config cleanup and the guard go in one small PR, together with #16303 (the mislabelled
codeql-actionversion comment insecurity.yml, the same CodeQL scope).Status, and a finding that changes the order of the last step. ACs (b) and (c) landed in #16311 (
b3983d696). Only (a) remains, and only for alert #1030. The other reviewed pairs are settled: #1042, #1045 and #1055 are dismissed with their reason, andupload_security.pyhas no alert in any state.The documented reason for #1030 doesn't match the current sink.
codeql-config.yml's first pair says the URL "is validated via_validate_git_url()". That's the git-clone path,import_git_repo. But #1030's current instance onDev_new_gui(.github/workflows/codeql.yml:analyzeata12b4bf79,external_importer.py:329) is the HTTP catalog fetch:session.get(url, …)inimport_http_catalog. Dismissing with the documented text would put a false reason on the record.The catalog sink's real guard, verified in code:
_validate_catalog_urlrequires an http(s) scheme and a hostname, then callsautobot_shared.url_safety.is_public_url_async. That rejects loopback, link-local (including 169.254.169.254), multicast, reserved and unspecified addresses, checks IPv4-mapped IPv6, and blocks RFC 1918 and IPv6 unique-local ranges (_ip_is_hard_blocked,_ip_is_private).autobot_shared.security.ssrf_guard.pinned_connectorpins the resolved public IP, which defeats DNS rebinding.- Redirects are disabled (
allow_redirects=False).
For internal-network SSRF, that makes it a genuine false positive.
Order from here:
- A PR corrects the config's reason for this pair to the catalog guard above. In the same PR: retire the non-functional
.github/codeql/python-exclusions.qls, fix the stale comments incodeql.ymlandssrf_guard.py, and add a guard that fails on any.github/codeql/file nothing references (ci(security): codeql.yml is disabled, so the reviewed codeql-config.yml exclusions apply to nothing #16283 AC3). - Then dismiss feat: Voice conversation mode — Hands-free with VAD (Tier 2) #1030 as a false positive, citing the corrected config text.
Being checked separately: whether the catalog routes (
GET /api/skills/catalog,POST /api/skills/catalog/{name}/install) require authentication. Neither handler declares an auth dependency. That's a different question from SSRF and doesn't change the dismissal reason, but if they're open it's its own finding and will be filed.The auth question from my last comment is answered, and it's filed as #16368 (security, critical). At the application layer, every route in
api/skills.pyis reachable without authentication, and the router-auth guards read only the core router registry. The SSRF false-positive reasoning for #1030 still stands: the catalog fetch is limited to public, pinned addresses whatever the auth. So #1030's dismissal proceeds as planned, citing the corrected config reason, and #16368 is tracked separately.- added a commit that references this issue
on Sep 13, 2026 Verification pass against merged
origin/main. AC2 and AC3 are both delivered; only AC1 remains, and it is the owner decision this issue already framed as one. No code needed from me — recording the evidence so it is not re-implemented.-
AC2 — the config and the doc no longer claim file-scoped
query-filterssuppress anything.
.github/codeql/codeql-config.yml:19-22now states it directly: "Anexclude:namingfiles:orpaths:matches no query", and:35-36records the history — "These used to bequery-filtersentries carryingfiles:/paths:, which never suppressed anything", with the reasons kept as prose rather than deleted.
docs/developer/CODEQL_SUPPRESSION.md:36corrects the table row that was wrong: "query-filtershides that query everywhere;paths-ignorehides every query in that path. Neither scopes one query to one file (ci(security): codeql-config.yml's file-scoped query-filters exclude nothing, under either setup #16302)", and:59-62scopes the guidance accordingly —query-filtersonly when a query is an FP everywhere,paths-ignoreonly when a path should not be analysed by any query. -
AC3 — a new file-scoped entry cannot be added and silently do nothing again.
repo_tests/codeql_query_filters_match_queries_16302_test.pyenforces it, and enforces it on behaviour rather than on the config's current text:test_a_file_scoped_exclude_is_rejected— parametrised over thefiles:andpaths:spellings, and asserts the offending key is named in the problem, so the failure tells an author what to change.test_every_documented_constraint_key_is_accepted— the other direction, so the guard cannot pass by rejecting everything.test_every_live_query_filter_matches_query_metadata— runs against the real config, so the file itself stays conformant.- Plus
not a list,no constraint, andreason beside the instructioncases.
That combination is what makes it a guard rather than a snapshot: it would fail on the exact three entries this issue was filed about.
-
AC1 — not met, and not mine to meet. The three reviewed pairs are still not demonstrably suppressed, because that requires choosing between the issue's own options 1/2/3: per-alert dismissal with documented reasons, SARIF filtering before upload, or
paths-ignore(which the issue recommends against, and I agree — it suppresses every query on those files, not the one reviewed).
My read, for whoever rules: option 1. It is what the
ssrf_guard.pyentry already did in practice, it works under either setup so it is not invalidated by #16283 moving between them, and it is per-alert, which is the granularity actually wanted — the whole defect here was a mechanism claiming a granularity it did not have. Option 2 keeps the exclusions as reviewed code in-repo, which is genuinely better provenance, but it adds a SARIF-rewriting step tocodeql.ymland a third-party action or local script to maintain, on a workflow that is already the slowest gate.Worth noting alongside #16858:
maincurrently carries 113 open alerts (1 critical, 104 high), so whichever mechanism is chosen, dismissals here should be the documented-reason kind rather than anything that makes the backlog smaller without anyone reading it.-
Same 3 branches as #16283 (this issue's own two,
issue-16302-codeql-cfg/issue-16302-codeql-leftovers, plus the sharedtrain/2026-09-13-batch1) — all superseded, checked the same way (file-level blob comparison at branch tip vsorigin/main); not reviving any.Verified all three ACs against current main:
- Each reviewed pair demonstrably suppressed.
py/full-ssrf/external_importer.py(feat: Voice conversation mode — Hands-free with VAD (Tier 2) #1030): fixed by a real code change (security(codeql): alert 1030 (py/full-ssrf, external_importer.py:329) is still open on main after #12278 closed, and re-reports on unrelated PRs #16595,fetch_safe_url) — see ci(security): codeql.yml is disabled, so the reviewed codeql-config.yml exclusions apply to nothing #16283.py/full-ssrf/ssrf_guard.py(fix: Voice conversation TTS 403 + overlay bubble rendering (#1029) #1042, feat(voice): hands-free mode with Silero VAD + Whisper transcription (#1030) #1045): confirmed viagh api— bothstate: dismissed,dismissed_reason: "false positive".py/path-injection/upload_security.py: zero open alerts of that rule against that path, confirmed viagh api repos/.../code-scanning/alerts?state=open. - Config and docs no longer claim file-scoped
query-filterssuppress anything.codeql-config.yml's current header states plainly: "These used to bequery-filtersentries carryingfiles:/paths:, which never suppressed anything. The reasons are the reviewed record; a dismissal comment cites the pair's issue." Landed via88738c9b86(ci(security): codeql.yml is disabled, so the reviewed codeql-config.yml exclusions apply to nothing #16283, ci(security): codeql-config.yml's file-scoped query-filters exclude nothing, under either setup #16302). - A new file-scoped
query-filtersentry can't silently do nothing again.repo_tests/codeql_query_filters_match_queries_16302_test.pyallowlists exactly the query-metadata keys CodeQL actually matches on (id,kind,tags,precision,problem.severity,query path/query filename,security-severity) and fails on anything else, plus fails on a missing/empty config so it can't read clean on nothing.
All three met with current code/API evidence. Closing.
- Each reviewed pair demonstrably suppressed.
Found while implementing #16283 (option 2, return to CodeQL advanced setup).
What
.github/codeql/codeql-config.ymlexcludes three reviewed false positives withquery-filtersentries that pair a query id with a file, usingfiles:orpaths:inside theexclude:block. A query filter matches on query metadata only:id,kind,tags,precision,problem.severityand similar. GitHub's advanced-setup customization docs send you to "Metadata for CodeQL queries" for what a filter can match, and a source file isn't query metadata. So none of the three entries is scoped to its file. My reading is that an exclude block naming a key no query carries matches no query at all, so each entry excludes nothing. The alert history below agrees.So returning to advanced setup (#16283) doesn't make the reviewed exclusions apply by itself.
Evidence
py/full-ssrf/autobot-backend/skills/external_importer.pyexclusion has been in the config since 717ec98 (2026-06-04).py/full-ssrf,autobot-backend/skills/external_importer.py) has instances under the advanced-setup analysis key.github/workflows/codeql.yml:analyze, category/language:python. They are open on at least 12 pull-request merge refs from July to August 2026, fromrefs/pull/12525/mergetorefs/pull/14164/merge. In that periodcodeql.ymlran withconfig-filepointing at this config.autobot_shared/security/ssrf_guard.pyrecords that its alerts (1042 and 1045) were handled by dismissal, not by the filter.Also inaccurate as a consequence
codeql-config.ymlsays aquery-filtersentry gives "per-path granularity".docs/developer/CODEQL_SUPPRESSION.mdhas a table row that saysquery-filters/paths-ignoresuppress per path.paths-ignoredoes, but for every query on the file.query-filtersdoesn't.Options (owner decision)
ssrf_guard.pyentry already records. Then either remove the three entries or turn them into reason-only comments, so the config stops claiming them.codeql.yml: run analyze withupload: never, drop results by rule id plus path, then upload withupload-sarif. This keeps the exclusions in the repo as reviewed code, at the cost of one more step, either a local script or a third-party action.paths-ignoreon the three files. This suppresses every query on those files, not just the one reviewed. I don't recommend it.Acceptance criteria
codeql-config.ymlandCODEQL_SUPPRESSION.mdno longer claim that file-scopedquery-filterssuppress anythingquery-filtersentry can't be added and silently do nothing again, whether through a guard or through removing that form from the documented options