chore(ci): Fix detection of changes to include models #10
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fx-detection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Code Review
Blocking Issues
None.
Suggestions
ssh-checkandssh-lockfilemissing fromclaude-reviewneeds (lines 649–676): Thessh-checkandssh-lockfilejobs are included in themerge-gatebut are absent fromclaude-review'sneedslist. This meansclaude-reviewcan run and post its comment before SSH job results are known. Since the merge gate captures all jobs, this doesn't allow a bypass — it's just a minor ordering inconsistency. Addingssh-checkandssh-lockfiletoclaude-review's needs would keep the pattern consistent with how other check/lockfile pairs are handled.CHANGED_FILESinjected into shell prompt string (lines 718–726): TheCHANGED_FILESvariable (fromgit diff --name-only) is interpolated directly into a multi-line shell string passed as the-pargument to claude. If a branch contains a file with a name containing shell metacharacters or newlines, this could produce unexpected prompt content. The risk is low (filenames come from the repo's own git index), but wrapping the changed files section in aprintf '%s' "$CHANGED_FILES"or using a temp file to compose the prompt would be more robust.deps-auditruns unconditionally (lines 611–634): Unlike every other job,deps-audithas noneeds: [changes]condition and no path filter — it runs on every PR regardless of what changed. This is intentional for security auditing, but it's worth confirming that thedeno outdatedscan across all lockfile directories (line 624) is fast enough not to slow down small PRs meaningfully.CI Security Review
Summary of Changes
This PR modifies
.forgejo/workflows/ci.ymlwith a two-line change: it removes| grep -v '^model/'from the changed-files computation in theclaude-review(line 697) andclaude-adversarial-review(line 814) jobs. This causesmodel/files to be included in the list of changed files passed to the Claude Code review agents.Analysis
1. Prompt Injection — No new risk introduced.
The changed lines affect which filenames are passed into the
CHANGED_FILESvariable that becomes part of the prompt. The filenames come fromgit diff --name-only origin/main..HEAD, which lists file paths from the repo's own git history — not from attacker-controlled event fields. The file contents are not interpolated into the prompt; only the file paths are listed, and the LLM reads file contents via tool calls. No new prompt injection surface.Note: The existing workflow already interpolates
CHANGED_FILES(a list of repo file paths) into the prompt string via shell variable expansion. This is acceptable because these are git-derived file paths from the checked-out repo, not attacker-controlled event metadata.2. Expression Injection & Script Injection — No change.
The PR does not introduce or modify any Actions expression interpolations in
run:blocks. The existing${{ matrix.task }}interpolations throughout the file use values from a hardcoded matrix (e.g.,[check, lint, fmt, test]), not from user input, so they are safe.3. Dangerous Triggers — No change.
The workflow triggers on
pull_request(notpull_request_target), which runs in the fork context without access to base repo secrets for external PRs. This is the safe pattern. No change in this PR.4. Supply Chain — No change.
All actions used are from trusted publishers (
actions/checkout@v6,denoland/setup-deno@v2). The Claude CLI binary is downloaded with a pinned version and verified via SHA-256 checksum — this is a good pattern. No changes to action references or supply chain in this PR.5. Permissions — No change.
Workflow-level permissions are
contents: read(line 9). Theclaude-review,claude-adversarial-review, andclaude-ci-security-reviewjobs override with job-levelpermissions: { contents: read, pull-requests: write }, which is appropriately scoped. No changes to permissions in this PR.6. Secret Exposure — No change.
ANTHROPIC_API_KEYandBOT_TOKENare passed viaenv:to specific steps that need them, not globally. The Claude CLI receives the API key through the environment variable, not via command-line argument. No changes to secret handling in this PR.7. Auto-merge & Trust Boundaries — No change.
The workflow does not auto-merge. The
merge-gatejob aggregates results but only fails/passes — it does not trigger merges. No changes to trust boundaries in this PR.Verdict
PASS — The change is security-neutral. It broadens the set of files reviewed by the Claude agents to include
model/directory files, which is a review-scope expansion, not a security regression. No new injection vectors, permission escalations, or secret exposure risks are introduced.