From ce9d41a4f8ab07b503d8c28d381f85fe7e0131ba Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:28:33 -0700 Subject: [PATCH 1/4] tests: state the insecure-fixture intent instead of sidestepping the sink The two negative-test fixtures were routed through Path.chmod because CodeQL models os.chmod and not the Path method. That resolved the alerts without expressing anything: the calls are behaviourally identical, and the choice depends on a scanner implementation detail that can change, at which point CI goes red on an unrelated PR and the reason has to be rediscovered. Use os.chmod with an inline suppression and a stated rationale at each site. The permissive mode is the input under test -- each assertion is that the resolver REFUSES the artifact -- and no production path sets a permissive mode. A CodeQL config filter was considered and rejected: query-filters match query metadata repository-wide, so excluding this query would unscan production too, and paths-ignore would drop every query across tests/ rather than this one. No path-scoped query filter exists, so the inline mechanism is the narrowest available. Co-Authored-By: Claude Opus 5 --- changelog.d/2026-07-30-codeql-test-fixture-suppression.md | 3 +++ tests/test_claude_transcript_model.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changelog.d/2026-07-30-codeql-test-fixture-suppression.md diff --git a/changelog.d/2026-07-30-codeql-test-fixture-suppression.md b/changelog.d/2026-07-30-codeql-test-fixture-suppression.md new file mode 100644 index 0000000..d51b051 --- /dev/null +++ b/changelog.d/2026-07-30-codeql-test-fixture-suppression.md @@ -0,0 +1,3 @@ +### Changed + +- Express the two deliberately-insecure test fixtures with `os.chmod` and an inline CodeQL suppression carrying a stated rationale, instead of routing them through `Path.chmod` to sidestep the query's modeled sink. The permissive mode in each is the input under test — the assertion is that the resolver refuses the artifact — and no production path sets a permissive mode. This states the intent where a reader will see it, rather than depending on which call shape the scanner happens to model. diff --git a/tests/test_claude_transcript_model.py b/tests/test_claude_transcript_model.py index 17cc688..b450727 100644 --- a/tests/test_claude_transcript_model.py +++ b/tests/test_claude_transcript_model.py @@ -167,7 +167,8 @@ 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 assertion IS that this is refused. - path.chmod(0o620) # group-write transcript + # The permissive mode is the input under test, not a defect. + os.chmod(path, 0o620) # codeql[py/overly-permissive-file] self.assertEqual(self._resolve(), ("invalid", "")) def test_hardlinked_transcript_is_rejected(self): @@ -199,7 +200,8 @@ 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. - loose.chmod(0o777) # world-write project directory + # The permissive mode is the input under test, not a defect. + os.chmod(loose, 0o777) # codeql[py/overly-permissive-file] self.assertEqual(self._resolve(), ("ok", "claude-opus-5")) def test_secure_duplicate_project_directory_is_not_skipped(self): From 0017b3f7cda614ffcc6a491755a7d528a0362e99 Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:38:33 -0700 Subject: [PATCH 2/4] tests: put the suppression rationale on the annotation line Cross-check nit: a same-line reason makes the suppression auditable by grep, rather than depending on a reader picking up the comment above it. Co-Authored-By: Claude Opus 5 --- tests/test_claude_transcript_model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_claude_transcript_model.py b/tests/test_claude_transcript_model.py index b450727..af2df35 100644 --- a/tests/test_claude_transcript_model.py +++ b/tests/test_claude_transcript_model.py @@ -168,7 +168,7 @@ def test_group_writable_transcript_is_rejected(self): path = self._write(_assistant("claude-opus-5")) # Deliberately insecure fixture: the assertion IS that this is refused. # The permissive mode is the input under test, not a defect. - os.chmod(path, 0o620) # codeql[py/overly-permissive-file] + os.chmod(path, 0o620) # codeql[py/overly-permissive-file] -- deliberate fixture; assertion is refusal self.assertEqual(self._resolve(), ("invalid", "")) def test_hardlinked_transcript_is_rejected(self): @@ -201,7 +201,7 @@ def test_group_writable_project_directory_is_skipped(self): planted.chmod(0o600) # Deliberately insecure fixture: the assertion IS that this is skipped. # The permissive mode is the input under test, not a defect. - os.chmod(loose, 0o777) # codeql[py/overly-permissive-file] + os.chmod(loose, 0o777) # codeql[py/overly-permissive-file] -- deliberate fixture; assertion is skip self.assertEqual(self._resolve(), ("ok", "claude-opus-5")) def test_secure_duplicate_project_directory_is_not_skipped(self): From 8c6ed97cbc0feb82ed9336e8b854e4f77fd0a07f Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:44:37 -0700 Subject: [PATCH 3/4] tests: record the CodeQL negative result at the call site The inline-suppression experiment failed. Reverting to os.chmod with `# codeql[py/overly-permissive-file]` annotations re-fired both alerts on the annotated lines (run 0017b3f), so GitHub code scanning does not honour inline CodeQL suppressions in this configuration. A config filter is not available either, for a different reason: 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. Keep Path.chmod and write the reason where it is used -- what the fixture is, why the permissive mode is the input under test, both mechanisms tried and why each failed, and not to tidy it back without re-checking CodeQL. This does not make the code safer; it was never unsafe. It turns an unexplained workaround into a documented decision, which was the actual complaint. Co-Authored-By: Claude Opus 5 --- ...6-07-30-codeql-test-fixture-suppression.md | 2 +- tests/test_claude_transcript_model.py | 25 +++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/changelog.d/2026-07-30-codeql-test-fixture-suppression.md b/changelog.d/2026-07-30-codeql-test-fixture-suppression.md index d51b051..0409429 100644 --- a/changelog.d/2026-07-30-codeql-test-fixture-suppression.md +++ b/changelog.d/2026-07-30-codeql-test-fixture-suppression.md @@ -1,3 +1,3 @@ ### Changed -- Express the two deliberately-insecure test fixtures with `os.chmod` and an inline CodeQL suppression carrying a stated rationale, instead of routing them through `Path.chmod` to sidestep the query's modeled sink. The permissive mode in each is the input under test — the assertion is that the resolver refuses the artifact — and no production path sets a permissive mode. This states the intent where a reader will see it, rather than depending on which call shape the scanner happens to model. +- Document why the two deliberately-insecure test fixtures set their modes through `Path.chmod` rather than `os.chmod`. CodeQL's `py/overly-permissive-file` models the `os.chmod` sink and flags them as high severity, but the permissive mode is the input under test — each assertion is that the resolver refuses the artifact — and no production path sets a permissive mode. The narrower mechanisms were tried and do not work: inline `# codeql[...]` suppressions are not honoured by GitHub code scanning in this setup, and a CodeQL config filter cannot be path-scoped, since `query-filters` match query metadata repository-wide and `paths-ignore` drops every query for the path. Recording this at the call site keeps a deliberate choice from reading as arbitrary style and from being tidied back into a red build. diff --git a/tests/test_claude_transcript_model.py b/tests/test_claude_transcript_model.py index af2df35..5845c47 100644 --- a/tests/test_claude_transcript_model.py +++ b/tests/test_claude_transcript_model.py @@ -166,9 +166,23 @@ 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 assertion IS that this is refused. - # The permissive mode is the input under test, not a defect. - os.chmod(path, 0o620) # codeql[py/overly-permissive-file] -- deliberate fixture; assertion is refusal + # 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[py/overly-permissive-file]` suppressions are NOT + # honoured by GitHub code scanning in this setup (verified + # empirically in PR #80: the alerts re-fired on the annotated lines); + # * a CodeQL config filter cannot be path-scoped -- `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. + # 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): @@ -200,8 +214,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. - # The permissive mode is the input under test, not a defect. - os.chmod(loose, 0o777) # codeql[py/overly-permissive-file] -- deliberate fixture; assertion is skip + # 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 f6d7f6842d070f6aab76865fbd61d45547ac8b46 Mon Sep 17 00:00:00 2001 From: John Osumi <931193+sumitake@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:50:58 -0700 Subject: [PATCH 4/4] tests: paraphrase the suppression grammar in the explanatory comment Cross-check re-consultation on the revised approach returned PROCEED-WITH-MODIFICATIONS with two additive points, both integrated. The comment explaining that inline suppressions do not work CONTAINED a suppression-shaped token. That is inert today, but a future parser with looser line or region rules could match it, and a suppression inventory or compliance audit would count it as an active directive. Describe the rule id in prose instead. Scope the config-filter claim to stock CodeQL config and name the heavier alternatives (split analysis jobs, custom packs, SARIF post-filtering) rather than asserting path scoping is impossible outright. Co-Authored-By: Claude Opus 5 --- ...6-07-30-codeql-test-fixture-suppression.md | 2 +- tests/test_claude_transcript_model.py | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/changelog.d/2026-07-30-codeql-test-fixture-suppression.md b/changelog.d/2026-07-30-codeql-test-fixture-suppression.md index 0409429..2c51a34 100644 --- a/changelog.d/2026-07-30-codeql-test-fixture-suppression.md +++ b/changelog.d/2026-07-30-codeql-test-fixture-suppression.md @@ -1,3 +1,3 @@ ### Changed -- Document why the two deliberately-insecure test fixtures set their modes through `Path.chmod` rather than `os.chmod`. CodeQL's `py/overly-permissive-file` models the `os.chmod` sink and flags them as high severity, but the permissive mode is the input under test — each assertion is that the resolver refuses the artifact — and no production path sets a permissive mode. The narrower mechanisms were tried and do not work: inline `# codeql[...]` suppressions are not honoured by GitHub code scanning in this setup, and a CodeQL config filter cannot be path-scoped, since `query-filters` match query metadata repository-wide and `paths-ignore` drops every query for the path. Recording this at the call site keeps a deliberate choice from reading as arbitrary style and from being tidied back into a red build. +- Document why the two deliberately-insecure test fixtures set their modes through `Path.chmod` rather than `os.chmod`. CodeQL's `py/overly-permissive-file` models the `os.chmod` sink and flags them as high severity, but the permissive mode is the input under test — each assertion is that the resolver refuses the artifact — and no production path sets a permissive mode. The narrower mechanisms were tried and do not work: inline codeql suppression comments are not honoured by GitHub code scanning in this setup, and stock CodeQL config cannot scope a filter to a path, since `query-filters` match query metadata repository-wide and `paths-ignore` drops every query for the path. Recording this at the call site keeps a deliberate choice from reading as arbitrary style and from being tidied back into a red build. diff --git a/tests/test_claude_transcript_model.py b/tests/test_claude_transcript_model.py index 5845c47..9e2801a 100644 --- a/tests/test_claude_transcript_model.py +++ b/tests/test_claude_transcript_model.py @@ -173,13 +173,18 @@ def test_group_writable_transcript_is_rejected(self): # 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[py/overly-permissive-file]` suppressions are NOT - # honoured by GitHub code scanning in this setup (verified - # empirically in PR #80: the alerts re-fired on the annotated lines); - # * a CodeQL config filter cannot be path-scoped -- `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. + # * 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