Skip to content

ci: commit security-review path scope to .github/claude-security-paths - #1701

Merged
kyle-sexton merged 1 commit into
mainfrom
chore/claude-security-paths-file
Jul 28, 2026
Merged

ci: commit security-review path scope to .github/claude-security-paths#1701
kyle-sexton merged 1 commit into
mainfrom
chore/claude-security-paths-file

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

No linked issue

Summary

Commits the repo's security-review path scope as .github/claude-security-paths — the caller's 26-entry inline paths list from .github/workflows/claude-security-review.yml, migrated verbatim (same patterns, same order) with a header comment stating the file's contract.

Sequencing safety, per the ci-workflows reusable's input descriptions (melodic-software/ci-workflows .github/workflows/claude-security-review.yml@main):

  • Inert now. The live caller passes the list inline via paths, and a non-empty paths input wins over paths-file, so this file has no effect until the caller changes.
  • Load-bearing later. The upcoming fleet-synced caller replaces the hand-written workflow with a component that passes only paths-file: .github/claude-security-paths, read from the PR's BASE branch. An absent file FAILS OPEN — every PR reviewed — so this file must land before that sync to keep the tuned scope.

The caller workflow itself is deliberately untouched; the sync replaces it.

Test plan

  • Scripted verbatim proof: parsed the caller's paths input with PyYAML, normalized both lists (comments/blanks stripped), compared — 26 inline entries vs 26 file entries, byte-identical, same order
  • File hygiene per .editorconfig/.gitattributes: LF only, single final newline, no trailing whitespace, UTF-8 without BOM
  • No workflow or other file changed (single-file diff)

Related

  • melodic-software/ci-workflows reusable claude-security-review.yml — owns the paths / paths-file precedence contract and names .github/claude-security-paths as the conventional location
  • Upcoming standards fleet sync that replaces this repo's hand-written security caller with the paths-file component

🤖 Generated with Claude Code

Materialize the caller's 26-entry inline `paths` list verbatim as the
repo-owned pattern file the ci-workflows claude-security-review reusable
reads via its `paths-file` input. Inert today: the caller still passes the
list inline, and a non-empty `paths` input wins over `paths-file`. It becomes
load-bearing when the fleet-synced caller (which passes only
`paths-file: .github/claude-security-paths`) replaces the hand-written one —
without this file that caller would fail open and review every PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 37s —— View job


PR Review — ci: commit security-review path scope to .github/claude-security-paths

  • Gather context
  • Read changed file
  • Verify path patterns match inline list
  • Check alignment with project conventions
  • Post findings

Summary

Single-file addition: .github/claude-security-paths — a preparatory scope file for the upcoming paths-file-based fleet-synced caller.

Verdict: looks good, no blocking issues.


Findings

Pattern fidelity — confirmed correct.
Manually compared the 26 entries in the new file (lines 6–31) against the 26 inline entries in .github/workflows/claude-security-review.yml (lines 37–62). Order and content are identical byte-for-byte. The PR description's PyYAML-based proof is consistent with the visual diff.

Sequencing safety — sound.
The PR body is accurate: the reusable's paths input takes precedence over paths-file when non-empty, so this file is genuinely inert until the caller is replaced. Committing the scope file first is the correct order; the fail-open note in the header is accurate and important for future maintainers.

Self-coverage — correct.
.github/** (line 6) covers .github/claude-security-paths itself, so any future scope change will trigger a security review. This is the right behavior.

File hygiene.
The .editorconfig [*] rule (LF, final newline, no trailing whitespace, UTF-8) applies to extensionless files. The file terminates cleanly at line 31 with a single trailing newline, no CRLF, no BOM. Compliant.

Dual MCP naming — deliberate and correct.
The two entries **/.mcp.json and **/mcp.json cover both naming conventions in the wild. No concern.

Header comment — accurate and useful.
The contract note (paths-file input, BASE branch read, fail-open behavior, format spec) is exactly what a future maintainer needs before editing this file. Well done.


Observations (non-blocking)

  • No **/*.py coverage — Python files aren't included, which is consistent with the inline list. If Python scripts are added to the repo later (e.g. in scripts/), scripts/** still catches them, so this isn't a gap for the current layout.
  • plugins/*/.claude-plugin/plugin.json is partially redundant with plugins/*/skills/** etc., but the plugin manifest is significant enough to call out explicitly, and redundancy in a path-scope file is harmless.

@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 30s —— View job


Security Review — PR #1701

  • Gather context and understand request
  • Read changed file
  • Analyze for security issues
  • Post findings

This PR adds a single text data file (.github/claude-security-paths) defining path scope for the Claude security review CI workflow. No code executes; the file is pure configuration consumed by an external reusable workflow.

Trust-boundary assessment

The key security property the PR description claims — that the consuming workflow reads this file from the BASE branch, not the PR head — is the critical invariant. If that holds, a contributor cannot modify this file within a PR to shrink the review scope for that same PR. The PR correctly identifies this as load-bearing: the file only takes effect when the fleet-synced caller switches to paths-file; the current inline paths wins now. That sequencing is sound.

The fail-open absent-file behavior (every PR reviewed when the file is missing) is the correct secure default. Removing the file makes the gate wider, not narrower.

Pattern coverage

plugins/*/hooks/** and the other plugins/*/ globs use a single-level *, which matches exactly one directory level. This correctly reflects the flat plugin layout (plugins/<name>/hooks/...) and is not a gap for the current structure.

.github/** covers this file itself, so any future modification to the review scope will itself be subject to security review. Self-referential — good.

**/*.yml / **/*.yaml are not in the list. Workflow files under .github/ are covered by .github/**, and plugin manifests are covered by plugins/*/.claude-plugin/plugin.json. If this repo ever adds YAML-based plugin configuration outside .github/, those files would be outside the review gate. No current exposure; worth bearing in mind as the plugin format evolves.

Findings

No security vulnerabilities found. The file introduces no executable surface, no secrets, no trust delegation, and no permission widening. The fail-open default and BASE-branch-read semantics are the right architecture for a scope gate of this kind. Nothing here for static analysis to have missed, and nothing I can find through logic or data-flow reasoning.

@kyle-sexton
kyle-sexton merged commit 1c0102a into main Jul 28, 2026
33 checks passed
@kyle-sexton
kyle-sexton deleted the chore/claude-security-paths-file branch July 28, 2026 02:04
kyle-sexton added a commit that referenced this pull request Jul 30, 2026
The reviewed runner-policy contract for claude-security-review.yml at
ci-workflows v0.9.1 declares allowedInputs ["runner", "paths-file"] — the
inline `paths` input the caller still passed is absent from it, so the
managed runner-policy lane failed the branch and would have reddened the
required ci-status aggregate.

`.github/claude-security-paths` already carries this exact pattern set
(#1701 committed it ahead of the pin bump that enables `paths-file`) and
nothing referenced it until now. Point the caller at it.

Job-level relevance gating is unchanged: `paths-file` is an input to the
reusable, not a workflow-level path filter, so an out-of-scope PR still
yields a name-stable skipped check rather than a Pending required one.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 30, 2026
… the review lane (#1766)

## Summary

Re-pins both locally-owned Claude lane callers from two divergent stale
pins (review `e295107`, security `66073e5`) to ci-workflows v0.9.1
(`c136b27f404dd32ce3873f39a6f3443891d1c16e # v0.9.1`), which makes both
lanes reachable by the org kill-switch variables
(`CLAUDE_LANES_DISABLED` etc.) for the first time. Three commits:

- Re-pin both callers; delete the explicit `skip-actors` lists so the
reusable's upstream-reviewed 4-actor default applies (widens the skip
set by `claude[bot]` + `melodic-ai[bot]` — ADR 0002 re-deliberation
filed as a follow-up issue).
- Migrate the security caller's inline `paths` to `paths-file:
.github/claude-security-paths` — required by the managed runner-policy
contract at v0.9.1 (`allowedInputs: ["runner","paths-file"]`); the file
pre-existed on main (#1701) and is order- and set-identical (26
patterns), so review scope is unchanged.
- Add the managed component's job-level serializer to the review caller
(`group: claude-review-${{ github.repository }}`, `queue: max`) — this
repo generates 52% of fleet lane volume and was the only consumer
without it. Security caller concurrency untouched (per-PR group, correct
for a required check).

Known: the `security-review / security-review` required check reports
green on THIS PR without running a review (workflow-touching PRs cannot
be reviewed by their own new caller; the action exits 0 on refusal). The
diff was independently verified instead — fresh-context verifier, 9/9
criteria PASS, including control tests proving the runner-policy checker
reads both files, byte-identical pin lines, and a no-deadlock analysis
of the concurrency groups.

## Test plan

- Runner-policy checker: exit 0 on the branch (base also 0; interim
`paths`-passing state correctly failed it — the contract works).
- actionlint clean under the repo's managed suppression (which is
load-bearing for `queue: max` — rhysd/actionlint#654).
- paths-file equivalence: 26/26 patterns, order-identical, set-identical
vs the removed inline list.
- Post-merge: next code-touching PR should produce an actual security
run at the new pin — tracked as Phase 3 closure evidence in ci-workflows
PLAN.md.

## Related

No linked issue: this PR closes no GitHub issue. Related: #1767 (ADR
0002 re-deliberation follow-up, opened by this change), #1701 (committed
the paths file this PR activates), ci-workflows
`docs/topics/claude-review-lanes/PLAN.md` Phase 3 (kill-switch
reachability + closure evidence).

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant