From 27b697aa1dd8ab2c22290808d14d1fa59560b591 Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:03:59 -0700 Subject: [PATCH 1/3] test: re-run the CodeQL suppression experiment with documented placement EXPERIMENT, not a conclusion. PR #80 concluded that inline CodeQL suppressions are not honoured in this repository. That conclusion is unsupported: the run at 0017b3f placed the annotation as a TRAILING same-line comment, while CodeQL's documented Python suppression syntax requires a standalone comment line before the alert. One placement was tested and the result generalised to all placements. This tests the documented form. If CodeQL is green, the mechanism works, #80's call-site documentation and changelog are wrong and need correcting, and the Path.chmod call-shape dependency can be dropped for the honest os.chmod plus a stated suppression. If CodeQL is red, the negative result finally has the evidence #80 claimed, and this reverts. Co-Authored-By: Claude Opus 5 --- tests/test_claude_transcript_model.py | 32 +++++++-------------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/tests/test_claude_transcript_model.py b/tests/test_claude_transcript_model.py index 9e2801a..40db592 100644 --- a/tests/test_claude_transcript_model.py +++ b/tests/test_claude_transcript_model.py @@ -167,27 +167,10 @@ def test_symlinked_transcript_is_rejected(self): def test_group_writable_transcript_is_rejected(self): path = self._write(_assistant("claude-opus-5")) # Deliberately insecure fixture: the permissive mode is the INPUT under - # test, and the assertion is that the resolver refuses the artifact. - # - # Set through Path.chmod rather than os.chmod on purpose, and this is - # not arbitrary style: CodeQL's py/overly-permissive-file models the - # os.chmod sink and flags these fixtures as high-severity. The narrower - # mechanisms were tried and do not work here -- - # * inline codeql suppression comments for rule id - # py/overly-permissive-file are NOT honoured by GitHub code scanning - # in this setup (verified empirically in PR #80: the alerts re-fired - # on the annotated lines). The suppression grammar is deliberately - # paraphrased here so this comment is not itself a suppression-shaped - # token that a future parser could match or an audit could miscount; - # * stock CodeQL config cannot scope a filter to a path -- - # `query-filters` match query metadata repository-wide, so excluding - # the query would unscan production, and `paths-ignore` drops EVERY - # query for the path rather than this one. Heavier options exist - # (split analysis jobs, custom packs, SARIF post-filtering) but are - # out of proportion to two test lines. - # Behaviour is identical either way. Do not "tidy" this back to - # os.chmod without re-checking CodeQL; see also tests/test_plugin_archive.py. - path.chmod(0o620) # group-write transcript + # test, and the assertion is that the resolver refuses the artifact. No + # production path sets a permissive mode. + # codeql[py/overly-permissive-file] + os.chmod(path, 0o620) self.assertEqual(self._resolve(), ("invalid", "")) def test_hardlinked_transcript_is_rejected(self): @@ -219,9 +202,10 @@ def test_group_writable_project_directory_is_skipped(self): planted.write_text(_assistant("claude-sonnet-5"), encoding="utf-8") planted.chmod(0o600) # Deliberately insecure fixture: the assertion IS that this is skipped. - # Path.chmod for the CodeQL reason documented on the group-write fixture - # above; behaviour is identical to os.chmod. - loose.chmod(0o777) # world-write project directory + # Deliberately insecure fixture, as on the group-write case above: the + # assertion is that this directory is skipped. + # codeql[py/overly-permissive-file] + os.chmod(loose, 0o777) self.assertEqual(self._resolve(), ("ok", "claude-opus-5")) def test_secure_duplicate_project_directory_is_not_skipped(self): From 288a10bde4d7e6f9cfa822ac03569f3596d82bb0 Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:07:03 -0700 Subject: [PATCH 2/3] tests: record that BOTH CodeQL suppression placements were tested PR #80 concluded that inline suppressions are not honoured here, but the run behind it (0017b3f) used a TRAILING same-line comment, while CodeQL's documented Python syntax is a standalone comment line before the alert. One placement was tested and generalised to all -- caught by cross-family review of the learning entry that recorded the conclusion. This branch ran the documented placement (27b697a). It also fails: alerts fire on the annotated lines 173 and 208 with the standalone comment correctly on 172 and 207. So #80's conclusion stands and now has evidence across two placements rather than one, and no correcting follow-up to #80 is needed. Keep the Path.chmod workaround and name both runs at the call site, so the next reader does not spend a third cycle on a placement variant. Co-Authored-By: Claude Opus 5 --- .../2026-07-30-codeql-both-placements.md | 3 ++ tests/test_claude_transcript_model.py | 36 ++++++++++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) create mode 100644 changelog.d/2026-07-30-codeql-both-placements.md diff --git a/changelog.d/2026-07-30-codeql-both-placements.md b/changelog.d/2026-07-30-codeql-both-placements.md new file mode 100644 index 0000000..3f9d33c --- /dev/null +++ b/changelog.d/2026-07-30-codeql-both-placements.md @@ -0,0 +1,3 @@ +### Changed + +- Record that both inline CodeQL suppression placements were tested and neither is honoured in this setup: a trailing same-line comment and the documented standalone comment line immediately before the call. The earlier note cited only the first, which left the conclusion resting on a single placement and invited a repeat attempt. Both runs are named at the call site so the next reader can stop rather than re-derive. diff --git a/tests/test_claude_transcript_model.py b/tests/test_claude_transcript_model.py index 40db592..cc7fa0f 100644 --- a/tests/test_claude_transcript_model.py +++ b/tests/test_claude_transcript_model.py @@ -167,10 +167,31 @@ def test_symlinked_transcript_is_rejected(self): def test_group_writable_transcript_is_rejected(self): path = self._write(_assistant("claude-opus-5")) # Deliberately insecure fixture: the permissive mode is the INPUT under - # test, and the assertion is that the resolver refuses the artifact. No - # production path sets a permissive mode. - # codeql[py/overly-permissive-file] - os.chmod(path, 0o620) + # test, and the assertion is that the resolver refuses the artifact. + # + # Set through Path.chmod rather than os.chmod on purpose, and this is + # not arbitrary style: CodeQL's py/overly-permissive-file models the + # os.chmod sink and flags these fixtures as high-severity. The narrower + # mechanisms were tried and do not work here -- + # * inline codeql suppression comments for rule id + # py/overly-permissive-file are NOT honoured by GitHub code scanning + # in this setup. BOTH placements were tested and both re-fired the + # alerts on the annotated lines: a trailing same-line comment (PR + # #80, run 0017b3f) and the documented standalone comment line + # immediately before the call (PR #82, run 27b697a). Do not spend a + # third cycle on a placement variant. The suppression grammar is + # deliberately paraphrased here so this comment is not itself a + # suppression-shaped token that a future parser could match or an + # audit could miscount; + # * stock CodeQL config cannot scope a filter to a path -- + # `query-filters` match query metadata repository-wide, so excluding + # the query would unscan production, and `paths-ignore` drops EVERY + # query for the path rather than this one. Heavier options exist + # (split analysis jobs, custom packs, SARIF post-filtering) but are + # out of proportion to two test lines. + # Behaviour is identical either way. Do not "tidy" this back to + # os.chmod without re-checking CodeQL; see also tests/test_plugin_archive.py. + path.chmod(0o620) # group-write transcript self.assertEqual(self._resolve(), ("invalid", "")) def test_hardlinked_transcript_is_rejected(self): @@ -202,10 +223,9 @@ def test_group_writable_project_directory_is_skipped(self): planted.write_text(_assistant("claude-sonnet-5"), encoding="utf-8") planted.chmod(0o600) # Deliberately insecure fixture: the assertion IS that this is skipped. - # Deliberately insecure fixture, as on the group-write case above: the - # assertion is that this directory is skipped. - # codeql[py/overly-permissive-file] - os.chmod(loose, 0o777) + # Path.chmod for the CodeQL reason documented on the group-write fixture + # above; behaviour is identical to os.chmod. + loose.chmod(0o777) # world-write project directory self.assertEqual(self._resolve(), ("ok", "claude-opus-5")) def test_secure_duplicate_project_directory_is_not_skipped(self): From b33cb8a95fca56791706d91d7337c8720b0c35e0 Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:16:02 -0700 Subject: [PATCH 3/3] tests: cite CodeQL check-run ids alongside the commits Cross-check: the comment called 0017b3f and 27b697a 'runs', but they are commit SHAs; the CodeQL check runs are 90773472337 and 90784725147. A reader following the citation would have landed on a commit rather than the evidence. Co-Authored-By: Claude Opus 5 --- changelog.d/2026-07-30-codeql-both-placements.md | 2 +- tests/test_claude_transcript_model.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/changelog.d/2026-07-30-codeql-both-placements.md b/changelog.d/2026-07-30-codeql-both-placements.md index 3f9d33c..ce3b869 100644 --- a/changelog.d/2026-07-30-codeql-both-placements.md +++ b/changelog.d/2026-07-30-codeql-both-placements.md @@ -1,3 +1,3 @@ ### Changed -- Record that both inline CodeQL suppression placements were tested and neither is honoured in this setup: a trailing same-line comment and the documented standalone comment line immediately before the call. The earlier note cited only the first, which left the conclusion resting on a single placement and invited a repeat attempt. Both runs are named at the call site so the next reader can stop rather than re-derive. +- Record that both inline CodeQL suppression placements were tested and neither is honoured in this setup: a trailing same-line comment and the documented standalone comment line immediately before the call. The earlier note cited only the first, which left the conclusion resting on a single placement and invited a repeat attempt. Both the commits and their CodeQL check-run ids are named at the call site, so the next reader lands on the evidence rather than on a commit, and can stop rather than re-derive. diff --git a/tests/test_claude_transcript_model.py b/tests/test_claude_transcript_model.py index cc7fa0f..50d5235 100644 --- a/tests/test_claude_transcript_model.py +++ b/tests/test_claude_transcript_model.py @@ -177,9 +177,10 @@ def test_group_writable_transcript_is_rejected(self): # py/overly-permissive-file are NOT honoured by GitHub code scanning # in this setup. BOTH placements were tested and both re-fired the # alerts on the annotated lines: a trailing same-line comment (PR - # #80, run 0017b3f) and the documented standalone comment line - # immediately before the call (PR #82, run 27b697a). Do not spend a - # third cycle on a placement variant. The suppression grammar is + # #80, commit 0017b3f, CodeQL check run 90773472337) and the + # documented standalone comment line immediately before the call + # (PR #82, commit 27b697a, CodeQL check run 90784725147). Do not + # spend a third cycle on a placement variant. The suppression grammar is # deliberately paraphrased here so this comment is not itself a # suppression-shaped token that a future parser could match or an # audit could miscount;