fix: clear the CodeQL findings on the scanner's own code - #66
Merged
Conversation
A security tool with open static-analysis findings on its own repository is making the argument against itself. Sixteen of seventeen are resolved here; the seventeenth is a deliberate design, dismissed with the reason recorded rather than silenced. Log forging (2, medium): the workflow file name arrives in the request and was logged verbatim on the OpenAI failure paths. Structured sinks keep it a property, but console rendering is a text line, and one embedded line break is enough to forge a log entry. Control characters are stripped once, at the top. Possible null dereference (1, warning): handlers passed request! after ValidateWorkflowRequest had established non-null somewhere the analyzer cannot see. The null test now lives at each call site where the compiler can see the proof, the helper takes a non-null request, and every null-forgiving operator on the request is gone. Nested ifs (1, note): flattened to one condition in the API-key middleware. Path.Combine (12, note): the eval and test fixture paths move to Path.Join, which does not silently discard the base when a later segment is rooted. 204 tests pass, 0 warnings. Co-Authored-By: Claude Opus 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.
The Security tab showed 17 open code-scanning alerts — on a security tool. Sixteen are fixed here; the seventeenth is deliberate and gets dismissed with its reason written down, which is what a triage is.
cs/log-forging\nforges a log entry. Control characters stripped once at the top ofExplainAsync.cs/dereferenced-value-may-be-nullrequest!after the shared validator established non-null where the analyzer cannot see it. The null test now lives at each call site, the helper takes non-null, every!on the request is gone — the compiler proves what the assertion used to claim.cs/nested-if-statementscs/path-combinePath.Join.cs/catch-of-all-exceptions/api/github/statusexists to convert any failure — HTTP, JWT signing, key parsing — into a structured "not connected" status. Narrowing the catch means the next unanticipated exception type turns a status probe into a 500.Also of note:
zeroDependabot vulnerabilities,zerosecret alerts,zeroadvisories — the 17 badge was all CodeQL.204 tests pass, 0 warnings. Alerts auto-close on the next CodeQL analysis of
main.