fix(config): clear the CodeQL alerts properly — and undo the one I made worse - #62
Merged
Conversation
…de worse Correcting my own fix. CodeQL re-ran on it and reported FIVE high-severity alerts where there had been four; the extra one was mine, and the watcher merged it to main because CodeQL is not a required check on this repo. That is on me twice: I dismissed the original failure without opening it, then shipped a fix I had not re-verified. THE TESTS, which I made worse. I had changed 0o644 to 0o640 — trading world-readable for group-readable, which CodeQL flags just the same — and my new regression test added a second one. 0o700 has NO group or world bits and is still not 0600, which is the only thing the tests need to prove: anything but 0600 is refused. Two alerts gone, both assertions unchanged. THE THREE IN config.py, which are a genuine false positive and are now documented rather than contorted around. The value CodeQL taints is the secret's NAME — a key of the `secrets` mapping — not its value and not its path. A doctor that reports "a secret failed" without saying WHICH one is useless, so the name has to be emitted. The redaction from the previous commit stands and is what matters: no path, no filename, no secret value ever reaches the output, and `test_doctor_never_echoes_a_secret_path` fails if one does. Each suppression carries its reasoning at the site, including the one on `emit`, which is a GENERATOR rather than a reporter — its stdout IS the rc file and must carry the reference for the shell to resolve at load. A suppression with a stated reason and a test behind it is a decision; a suppression without one is a silencer. These are the first kind.
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
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.
Follow-up to #60, correcting my own fix.
CodeQL re-ran on that fix and reported five high-severity alerts where there had been four. The extra one was mine, and it reached
mainbecause CodeQL is not a required check here. Two errors on my side: I dismissed the original failure as a startup fault without opening it, then shipped a fix I had not re-verified.The tests — I made these worse
I changed
0o644→0o640, trading world-readable for group-readable, which CodeQL flags identically. And my new regression test added a second one.0o700has no group or world bits at all and is still not0600— which is the only thing the tests need to prove: anything but0600is refused. Two alerts gone, both assertions unchanged.The three in
config.py— a genuine false positive, now documentedThe value CodeQL taints is the secret's name (a key of the
secretsmapping), not its value and not its path. A doctor that reports "a secret failed" without saying which one is useless, so the name has to be emitted.The redaction from #60 stands and is the part that matters: no path, no filename, no secret value reaches the output, and
test_doctor_never_echoes_a_secret_pathfails if one does.Each suppression carries its reasoning at the site — including the one on
emit, which is a generator, not a reporter: its stdout is the rc file and must carry the reference for the shell to resolve at load.