ci: optimize model checks, fix review sandbox, remove GitHub workflows #5
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "worktree-462"
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?
Summary
Optimizes CI for generated model type-checking, fixes the Claude Code review sandbox issue, removes legacy GitHub workflows, and adds a merge gate.
Closes #462
What changed
CI optimization (~30min → ~2min for model checks)
The
aws-check,gcp-check, andcloudflare-checkjobs previously iterated through all 586 service directories (255 AWS + 262 GCP + 69 Cloudflare) runningdeno check,deno lint, anddeno fmton every PR that touchesmodel/orcodegen/.fmtandlintfor all generated model jobs (AWS, GCP, Cloudflare, Hetzner, DigitalOcean). These are provably redundant: the generator already runsdeno fmton output, and every generated file includes// deno-lint-ignore-file no-explicit-any.deno checkwith sample-based checking. 13 representative services chosen for schema diversity:healthimaging(baseline),quicksight(deep nesting),sns(json/any unions),apigateway(array/map types),ec2(106 model files)groupssettings(flat baseline),container(1092 z.object calls),storage(z.any patterns),dataproc(153 z.array + 90 z.record),compute(119 model files)r2(simple),load-balancing(nested),pages(array/map-heavy)full-model-check.ymlthat runs fulldeno checkacross all services as a safety net.Claude Code review sandbox fix
All three review jobs (
claude-review,claude-adversarial-review,claude-ci-security-review) were writing output to$RUNNER_TEMP/review/which is outside the workspace. Claude Code's sandbox blocks writes outside the workspace, causing reviews to silently fail to produce output files. Moved the output directory to$GITHUB_WORKSPACE/.review-output/.Merge gate
Added a
merge-gatejob that depends on every check and review job. If any job fails (including reviews), the gate fails. Set this as the required status check in Forgejo branch protection to block merges on review failures.GitHub workflow removal
Removed all
.github/workflows/files (ci.yml,publish.yml,regenerate-models.yml,auto-response.yml). CI runs exclusively on Forgejo now; GitHub is a read-only mirror with issues disabled. Updated allagent-constraints/docs andscripts/audit_actions.tsto reference.forgejo/workflows/.Why this is correct
.tsfile contains// deno-lint-ignore-file no-explicit-any(verified via grep), and the codegen pipeline runsdeno fmton all output as part of generation.review-body.md,review-failed) moved into the workspace. The binary download, changed-files list, and payload assembly remain in$RUNNER_TEMP(written by shell steps outside the sandbox).contains(needs.*.result, 'failure')— skipped jobs (due to path filters not matching) don't count as failures, so the gate only blocks on actual failures.Test plan
full-model-check.ymlvia workflow_dispatch and confirm full check runs.review-output/(no sandbox errors)merge-gateas required status check in Forgejo branch protectionCI Security Review
Medium
.forgejo/workflows/ci.yml:632-633, 744-745, 835-836 —pull-requests: writepermission on GITHUB_TOKEN is unused. The three Claude review jobs (claude-review,claude-adversarial-review,claude-ci-security-review) declarepull-requests: writein their job-level permissions, but they post review comments using theBOT_TOKENsecret via curl, not viaGITHUB_TOKEN. Thepull-requests: writescope onGITHUB_TOKENis not consumed by any step. This is not exploitable since no step usesGITHUB_TOKENwith write intent, but removing it would tighten the principle of least privilege. Fix: Removepull-requests: writefrom these jobs'permissions:blocks, or reduce to justcontents: read.Positive Security Observations
Prompt injection mitigated: The Claude review steps construct prompts from
cat .forgejo/prompts/*.md(repo-controlled files) andCHANGED_FILES(file paths fromgit diff --name-only). No attacker-controlled event data (PR title, body, issue content, commit messages) is interpolated into the prompt. The LLM fetches PR content itself viagit difftool calls. All three prompt files include a security preamble instructing the model to treat PR content as untrusted.Tool scoping is tight: The
--allowedToolsfor all three Claude review steps are correctly minimal:Read,Grep,Bash(git diff:*),Bash(git log:*),Bash(tee ${OUTPUT_DIR}/review-body.md:*),Bash(touch ${OUTPUT_DIR}/review-failed). No arbitrary bash, noghCLI, no network access. This is a significant improvement over the deleted GitHub workflow which grantedBash(gh pr review:*)andBash(gh pr diff:*).Supply chain hardened for Claude CLI: The Claude CLI is downloaded with a pinned version (
2.1.150) and verified via SHA256 checksum before execution (.forgejo/workflows/ci.yml:665-668). This is equivalent to SHA-pinning an action.Actions are from trusted publishers: All referenced actions (
actions/checkout@v6,denoland/setup-deno@v2) are from trusted publishers, acceptable with tag-only pins per project policy.Workflow-level permissions are minimal: The top-level
permissions: contents: readapplies to all jobs by default. Only review jobs add write permissions at the job level (though as noted above, the write perm is unused).No dangerous triggers: Both Forgejo workflows use safe triggers (
pull_request,schedule,workflow_dispatch). Nopull_request_target, noissue_commenttriggers.Reduced attack surface: Deleting four GitHub Actions workflows (auto-response, ci, publish, regenerate-models) eliminates potential attack vectors, including the
issues: [opened]trigger inauto-response.ymland the PAT-basedcontents: write+ force-push inregenerate-models.yml.Expression injection safe: All GitHub/Forgejo expressions in
run:blocks are either from controlled matrix values (${{ matrix.task }},${{ matrix.vault }}), boolean checks (${{ contains(...) }}), or are passed through environment variables (BASE_SHA,HEAD_SHA) before use in shell commands.Secret handling is clean:
ANTHROPIC_API_KEYis scoped to only the Claude CLI steps.BOT_TOKENis scoped to only the comment-posting steps. Neither is logged or exposed through command substitution.Verdict
PASS — This PR is a net security improvement. It migrates from GitHub Actions (with
claude-code-actionand broader tool grants) to Forgejo Actions with tightly-scoped Claude CLI invocations, SHA256-verified downloads, and prompt-injection-resistant prompt construction. The only finding is an unusedpull-requests: writepermission that is not exploitable.Code Review
Blocking Issues
ssh/missing fromdeps-auditfind expression (.forgejo/workflows/ci.ymlline 579)The
deps-auditjob scans:The
ssh/directory is absent. SSH has its owndeno.lockand dedicatedssh-check/ssh-lockfileCI jobs, but its dependencies are never audited for known vulnerabilities or outdated packages. Per the CI fan-out checklist inplanning-conventions.mdand theci-matrix-completenessdimension inadversarial-dimensions.md, thedeps-auditfind expression must cover every extension directory.Fix: add
sshto the directory list on line 579.Suggestions
claude-reviewneeds array missingssh-checkandssh-lockfile(.forgejo/workflows/ci.ymllines 603–629)All other check/lockfile pairs are present, but
ssh-checkandssh-lockfileare omitted. This meansclaude-reviewcan post its verdict while SSH tests are still in-flight. Themerge-gatestill catches SSH failures, so nothing broken merges, but the ordering violates the CI fan-out convention documented inplanning-conventions.mdandadversarial-dimensions.md.claude-adversarial-reviewneeds array missing several jobs (.forgejo/workflows/ci.ymllines 719–742)The adversarial review's
needs:omitsmodel-check,model-lockfile,ssh-check,ssh-lockfile, andactions-audit. Same ordering concern as above — the merge-gate provides the final safety net, but the convention calls for these to be listed.SSH changes don't trigger
claude-adversarial-review(.forgejo/workflows/ci.ymlline 750)The condition that activates adversarial review checks
vaults,datastores,issue-lifecycle,workflows,cve, andcodegen, but notssh. If the SSH extension warrants the adversarialtest-fidelityandsdk-wrap-defensechecks (it connects to external hosts), addneeds.changes.outputs.ssh == 'true'to the condition.--allow-writeunscoped in theactions-auditstep (.forgejo/workflows/ci.ymlline 600)--allow-writegrants filesystem-wide write access; only$GITHUB_STEP_SUMMARYis actually needed.implementation-conventions.mdcitesaudit_actions.tsas the precedent for scoped permissions — the precedent should match the principle. Use--allow-write=$GITHUB_STEP_SUMMARY.audit_actions.tsversion checks against GitHub's releases API, but workflows run on Forgejo (scripts/audit_actions.tsline 251)The script hardcodes
workflowDir = ".forgejo/workflows"(correct after the GitHub workflow removal) and queriesapi.github.comto compare action versions. Forgejo's action catalog (e.g.code.forgejo.org) uses its own versioning —actions/checkout@v6exists there but GitHub's latest is v4.x.isSameMajor("v6", "v4.x.x")returnsfalse, so every PR's actions-audit output will carry a spurious "outdated" warning for the checkout action. This is a warning, notexit 1, so it won't block merges, but the persistent noise undercuts the signal value of the audit. Consider either skipping the version-outdated check for trusted publishers that are known to diverge between Forgejo and GitHub, or maintaining a known-version allowlist.CI Security Review
Critical / High
No critical or high severity findings.
Medium
.forgejo/workflows/ci.yml:632-633, 744-745, 835-836 —pull-requests: writepermission may be unnecessary on review jobs. Theclaude-review,claude-adversarial-review, andclaude-ci-security-reviewjobs each declarepull-requests: writeat the job level. However, none of these jobs use the built-inGITHUB_TOKENfor PR write operations — they post review comments viacurlto the Forgejo API using theBOT_TOKENsecret. If the Forgejo runner does not requirepull-requests: writefor some other purpose, this grants theGITHUB_TOKENbroader scope than needed. Suggestion: Verify whetherpull-requests: writeis required by the Forgejo runner for these jobs; if not, remove it to follow least-privilege.Low
No low severity findings.
Notes — Security-Positive Changes
.github/workflows/auto-response.yml: This workflow ran onissues: [opened]and processed attacker-controlled issue content (title, body) to forward to an external API. Removing it eliminates an external-user-triggered workflow that handled untrusted input..github/workflows/regenerate-models.yml: This workflow hadcontents: write+pull-requests: writepermissions, used a PAT (GH_TOKEN) for checkout and PR creation, and rangit push --force. Removing it reduces the surface of privileged automation..github/workflows/ci.ymland.github/workflows/publish.yml: Consolidation from GitHub Actions to Forgejo reduces the number of CI surfaces to audit and maintain.Read,Grep,Bash(git diff:*),Bash(git log:*), and tightly-scoped tee/touch commands. No arbitrary Bash, no network access, no write capabilities beyond the review output files.CHANGED_FILESvariable passed to Claude prompts contains only file paths fromgit diff --name-only, not PR title/body/comment content. TheBASE_SHA/HEAD_SHAvalues in the changes job are correctly passed via environment variables rather than shell interpolation.sha256sum -c), preventing supply chain tampering.${{ }}expressions inrun:blocks use either fixed matrix values (matrix.task,matrix.vault, etc.) or safe boolean expressions (contains(needs.*.result, 'failure')). No event-payload fields are interpolated in shell commands..forgejo/workflows/full-model-check.yml: Clean scheduled workflow with read-only permissions, no secrets, no sensitive operations.Verdict
PASS — The changes are security-neutral to security-positive. The migration from GitHub Actions to Forgejo consolidates CI surface area, deleted workflows remove privileged automation and external-trigger attack vectors, and the Forgejo workflows follow good security practices (least-privilege permissions, prompt hardening, tight tool scoping, integrity-verified dependencies).
Code Review
Blocking Issues
scripts/changes are excluded from CI security review scope (.forgejo/workflows/ci.yml,claude-ci-security-reviewjob, "Compute changed files" step)The
changesjob usescheck_path ci ".forgejo/workflows/" "scripts/", so ascripts/-only change correctly setsci=trueand triggers theclaude-ci-security-reviewjob. However, the changed-files computation in that job filters withgrep -E '^\.(forgejo|github)/', which excludesscripts/*.tsfrom the file list passed to Claude. If a PR only touchesscripts/(e.g.,audit_actions.ts),has_fileswill befalseand the review step is skipped silently — the security review job runs, does nothing, and reports success. Scripts running in CI with--allow-net=api.github.com --allow-writeare exactly the kind of code that the CI security review is meant to cover.Fix: Broaden the grep to also capture
scripts/files:Suggestions
Lint and fmt checks for AWS, GCP, and Cloudflare models are now unchecked in any CI — The PR removes
lintandfmtfrom the PR CI for all model providers, and the newfull-model-check.ymlonly adds them back forhetzner-cloudanddigitalocean. AWS, GCP, and Cloudflare generated code now has no CI lint/fmt coverage anywhere — neither on PR nor on schedule. Thehetzner-digitalocean-full-checkjob runsdeno lint --no-configanddeno fmt --no-config --check, but the AWS/GCP/Cloudflare full-check jobs do not. Consider adding lint/fmt steps to theaws-full-check,gcp-full-check, andcloudflare-full-checkjobs infull-model-check.ymlfor parity.Globtool absent from review--allowedTools— The old GitHub Actions CI providedRead,Glob,Grepplusghcommands. The new Forgejo implementation dropsGlob. Without it, the reviewer cannot list directory contents and must rely entirely on thegit diffoutput for structure discovery. AddingGlobto the--allowedToolsstring in all three review jobs (claude-review,claude-adversarial-review,claude-ci-security-review) would give Claude the same file-exploration capability it had before.changesjob failure silently bypasses all per-scope checks in the merge gate — Themerge-gatejob does not includechangesin itsneeds:list. If thechangesjob fails (e.g., checkout fails,git differrors), all per-scope check/lockfile jobs that depend on it are skipped rather than failed. Sincecontains(needs.*.result, 'failure')returns false for skipped jobs, the merge-gate passes despite no scope checks having run. Addingchangestomerge-gate'sneeds:would surface this failure explicitly.ssh-checkandssh-lockfileabsent fromclaude-reviewneeds:— For SSH-only PRs, the Claude review can start and complete before or without SSH tests finishing, sincessh-check/ssh-lockfileare not inclaude-review's dependency list. Themerge-gatebackstop prevents actual merging with failing tests, but peradversarial-dimensions.md, theclaude-review/ test ordering gap is the documented "worst case." This is low urgency given the merge gate, but worth aligning with the stated convention.CI Security Review
Summary of Changes
This PR migrates CI from GitHub Actions (
.github/workflows/) to Forgejo Actions (.forgejo/workflows/), deleting the four GitHub workflow files (auto-response.yml,ci.yml,publish.yml,regenerate-models.yml) and modifying the existing Forgejo CI (ci.yml) with:full-model-check.ymlfor daily scheduled full checksmerge-gatejob that aggregates all required job results$RUNNER_TEMPto$GITHUB_WORKSPACE/.review-outputscripts/sshdirectory added to outdated-dependency scanscripts/audit_actions.tsfor auditing action pins and versionsactions-auditjob added to CICritical / High
No critical or high severity findings.
Medium
Workflow-level permissions instead of job-level —
.forgejo/workflows/ci.yml:8-9:permissions: contents: readis set at the workflow level. This means all jobs inheritcontents: readeven if they don't need it. Jobs likeclaude-review,claude-adversarial-review, andclaude-ci-security-reviewoverride with job-level permissions (addingpull-requests: write), which is good. However, best practice is to setpermissions: {}at the workflow level and declare per-job. In practice, since the base is read-only and the jobs that need write permissions declare them explicitly, this is not exploitable — this is a hardening suggestion, not a blocking issue.Tag-only pins on trusted actions — Throughout both
.forgejo/workflows/ci.ymland.forgejo/workflows/full-model-check.yml,actions/checkout@v6anddenoland/setup-deno@v2use tag-based pins rather than commit SHAs. Per the project's own policy inscripts/audit_actions.ts, these are from trusted publishers (actions,denoland) and are explicitly acceptable with tag-only pins. This is consistent and intentional, but noting it for completeness.scripts/audit_actions.tsonly scans.forgejo/workflows/—scripts/audit_actions.ts:251: TheworkflowDiris hardcoded to.forgejo/workflows. Since the.github/workflows/files are being deleted in this PR, this is correct going forward. However, if any.github/workflows are ever re-added (e.g., the auto-response workflow was GitHub-specific), they would not be audited. Low risk since the GitHub workflows are being removed.Low
contains(needs.*.result, 'failure')in merge-gate —.forgejo/workflows/ci.yml:958: This expression is interpolated into arun:block via${{ }}. Sincecontains(needs.*.result, 'failure')evaluates to a boolean string (true/false) controlled entirely by the CI runtime (not by any user input), there is no injection risk here. This is a safe use of expression interpolation.Review output path moved to workspace — The review output was moved from
$RUNNER_TEMP/review/to$GITHUB_WORKSPACE/.review-output/. Both are ephemeral per-job. The new path is within the workspace directory, but since the Claude Code agent's tools are tightly scoped (Read,Grep,Bash(git diff:*),Bash(git log:*),Bash(tee ...),Bash(touch ...)), the agent cannot write arbitrary files to the workspace. This is a neutral change.Deleted workflows assessment — The four deleted
.github/workflows/files are being removed as part of the Forgejo migration:auto-response.yml: Was anissues: [opened]trigger workflow that forwarded GitHub issues to swamp.club lab. It usedactions/github-script@v7(tag-only pin, but from trustedactionspublisher) and properly scoped permissions (issues: write,contents: read). The issue body was passed through to a swamp.club API, but theactions/github-scriptcontext handles this safely via the GitHub REST API rather than shell interpolation. Deletion is clean.ci.yml: The old GitHub CI useddorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d(properly SHA-pinned third-party action). The new Forgejo CI replaces this with a custom shell-based change detection usinggit diffwith env vars for base/head SHA — this is a safe pattern that avoids expression injection by passing the SHAs via environment variables.publish.yml: Had properly scoped permissions (contents: read), triggered only onpushtomain. Deletion is clean.regenerate-models.yml: Hadcontents: writeandpull-requests: writepermissions, usedsecrets.GH_TOKENfor push access. The${{ inputs.provider }}was used in a shell context but only accepts values from a fixedchoiceenum, limiting injection risk. The${{ steps.label.outputs.title }}interpolation in thegit commitandgh pr createcommands was safe since it was derived from the fixed enum. Deletion removes these concerns entirely.Verdict
PASS — The changes are security-neutral to positive. The migration from GitHub Actions to Forgejo Actions is clean. LLM review jobs maintain tight tool scoping. No attacker-controlled data is interpolated into prompts or shell commands. Permissions are appropriately scoped. The new
audit_actions.tsscript adds proactive supply chain monitoring. The deletion of GitHub workflows removes the codebase's attack surface on that platform.Code Review
Blocking Issues
None.
Suggestions
AWS/GCP/Cloudflare model lint and fmt checks are now absent everywhere. The PR moves lint/fmt for Hetzner and DigitalOcean models to the nightly
full-model-check.yml— that workflow correctly addsdeno lint --no-configanddeno fmt --no-config --checksteps for those two providers. But the nightlyaws-full-check,gcp-full-check, andcloudflare-full-checkjobs only rundeno check, with no lint or fmt step. PR CI no longer runs lint/fmt for these providers (by design, for speed), and neither does the nightly. So a codegen change that emits non-conformant code for AWS/GCP/Cloudflare would go undetected indefinitely. Adding lint/fmt loops to those three nightly jobs (mirroring thehetzner-digitalocean-full-checkpattern) would close the gap without impacting PR CI speed.merge-gateonly catchesfailure, notcancelled. The expressioncontains(needs.*.result, 'failure')passes silently when a dependency iscancelled(distinct fromskipped). In the normal push-to-PR flow the entire run is cancelled and restarted, so this is low-risk. But if a runner goes down mid-run or a user manually cancels the workflow,claude-review(or another gated job) could be cancelled within the run while the gate still reports success. Considercontains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')for a stricter posture.No
.gitignoreentry for.review-output/. The Claude review jobs now writereview-body.mdandreview-failedinto${GITHUB_WORKSPACE}/.review-output(inside the repo checkout). No workflow step commits these files, so the practical risk is low. Adding.review-output/to.gitignorewould prevent them from being accidentally staged by a developer running the review prompt locally or in a modified workflow.audit_actions.tsscans only.forgejo/workflows. This is correct given the deletion of.github/workflows/files in this PR. Worth noting: if.github/workflows/files are ever re-added (e.g., for GitHub mirroring), they would be silently unaudited by theactions-auditCI job. A comment inmain()documenting the single-directory scope would help future readers understand this is intentional, not an oversight.