Repository navigation
ci: enforce egress-block on the three issue-triggered AI jobs - #602
Conversation
Flips `claude` (claude.yml), `implement` (claude-implement.yml) and `respond` (bestaxbot-reply.yml) from `egress-policy: audit` to `block`, each with the rule 10 assertion. This is the first half of #578; `claude-review` and `claude-pr-loop`'s `fix`/`verify` follow separately, because those three can be exercised from a branch before merge and these three cannot -- all of them trigger on `issues`/`issue_comment`, which always run the default-branch workflow. The allowlist is measured, not guessed. Each job's audit run was read out of its own `Post Harden runner` step, which dumps the agent log with every DNS resolution and connection in it -- so no StepSecurity UI transcription was involved, and the recipe is now written into rule 10 so the next list does not have to rediscover it. Runs: 33231511797 (claude), 33230068020 (implement), 33231488110 (respond). All three came out as the same six application hosts, and that set is a strict subset of the eight already enforcing on ai-scan/scan, ai-triage/triage and claude-repro/author -- so this adopts a proven list rather than inventing one. Two notes worth keeping: - #578 predicted `claude` would be materially narrower because it has no install step. It is not: `pnpm/action-setup` pulls pnpm from registry.npmjs.org either way. - `objects.githubusercontent.com` and `release-assets.githubusercontent.com` never appeared -- the Node 24 toolcache hit on every run. They stay for the toolcache-miss path, exactly as the three existing jobs carry them. Three observed hosts are deliberately left out. `telemetry.vercel.com` is turbo's telemetry, so it gets `TURBO_TELEMETRY_DISABLED: '1'` beside the existing `CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC` rather than a slot on the allowlist -- same call #577 made for the Datadog host. It appeared in only one of the five runs because turbo samples, which is precisely the intermittent that would redden a block job weeks from now. The Actions cache blob host cannot be pinned at all (productionresultssa3/6/7/9/11 across five runs), so a cache miss is accepted; it degrades to a registry.npmjs.org refetch. `private-user-images.githubusercontent.com` is `implement` reading an image in an issue body -- left out because on a public repo it is the one candidate host serving attacker-supplied bytes into a job holding bestaxbot's PAT. Every host was resolved before being listed: an unresolvable entry makes the agent revert the firewall while the pre-step still exits 0, which is how statsig.anthropic.com silently disabled the policy on #577's first run. The assertion steps are byte-identical to auto-close-duplicates.yml's apart from the credential each names, so the `-e`-check-first, polling and `^Initialized` shapes are all preserved rather than hand-rewritten. Rule 10's inventory moves the three jobs into "enforcing and asserted" and re-derives the count (13 -> 16 key-form, 14 -> 17 plain-form), and I1's egress-policy clause now says which jobs it actually covers instead of "once #578 lands". Refs #578
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview DeploymentPreview URL: https://2f55a65e.bestax.pages.dev |
There was a problem hiding this comment.
🟡 Changes recommended
The allowlists block setup-node’s nodejs.org fallback, and three comments incorrectly retain audit-mode documentation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR implements the first half of #578 by enforcing measured egress allowlists on three credentialed AI jobs.
Changes:
- Switches three workflows from audit to block mode with enforcement assertions.
- Disables Turbo telemetry.
- Updates workflow security guidance and inventory.
File summaries
| File | Description |
|---|---|
.github/workflows/claude.yml |
Enforces egress blocking for Claude sessions. |
.github/workflows/claude-implement.yml |
Enforces blocking for implementation jobs. |
.github/workflows/bestaxbot-reply.yml |
Enforces blocking for bestaxbot replies. |
.github/CLAUDE.md |
Documents allowlist measurement and updated coverage. |
Review details
Suppressed comments (3)
.github/workflows/claude.yml:221
- The surrounding
Run Claudedocumentation still says this is an audit job producing a future block allowlist, which is false after this PR switches it toblock. Update that explanation together with this telemetry setting so future maintainers do not treat this job as a measurement run.
# Same call, second vendor: turbo phones telemetry.vercel.com from the
# pnpm commands this session runs. It appeared in only one of the five
# audit runs because turbo samples, which is precisely the intermittent
# that reddens a block job weeks after a flip. Denied, not allow-listed
# (#578).
.github/workflows/claude-implement.yml:206
- The surrounding
Run Claudedocumentation still says this is an audit job producing a future block allowlist, which is false after this PR switches it toblock. Update that explanation together with this telemetry setting so future maintainers do not treat this job as a measurement run.
# Same call, second vendor: turbo phones telemetry.vercel.com from the
# pnpm commands this session runs. This is the job that measured it —
# once, in run 33230068020, because turbo samples, which is precisely
# the intermittent that reddens a block job weeks after a flip. Denied,
# not allow-listed (#578).
.github/workflows/bestaxbot-reply.yml:269
- The surrounding
Run Claudedocumentation still says this is an audit job producing a future block allowlist, which is false after this PR switches it toblock. Update that explanation together with this telemetry setting so future maintainers do not treat this job as a measurement run.
# Same call, second vendor: turbo phones telemetry.vercel.com from the
# pnpm commands this session runs. It appeared in only one of the five
# audit runs because turbo samples, which is precisely the intermittent
# that reddens a block job weeks after a flip. Denied, not allow-listed
# (#578).
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The first version of these comments predicted that omitting the Actions cache blob hosts from the allowlist would cost a cache miss, degrading to a registry.npmjs.org refetch. That was a guess and it was wrong. Run 33286967625 exercised claude-review at `egress-policy: block` on a branch, with neither `productionresultssa<N>.blob.core.windows.net` nor `results-receiver.actions.githubusercontent.com` allow-listed. `setup-node` restored the pnpm cache successfully and `pnpm install --frozen-lockfile` completed in 15.7s. harden-runner does not gate the runner's own control-plane traffic, so leaving those hosts out costs nothing at all. That run also produced `productionresultssa13`, a sixth distinct blob host name, which strengthens rather than changes the "cannot be pinned" argument. Replaces the predicted cost with the measured result in all three job comments and in rule 10's measurement section, and says plainly in the rule that the prediction was wrong -- the point of that section is that you read a run instead of reasoning about what a policy will cost.
Correction pushed: the cache claim was a prediction, and it was wrong
These comments said that omitting the Actions cache blob hosts costs a cache miss which "degrades harden-runner does not gate the runner's own control-plane traffic. Leaving those hosts off costs Rule 10's measurement section now states the mistake rather than quietly fixing it, because the Also folded in: |
There was a problem hiding this comment.
🔵 Needs a closer look
Three workflow comments still incorrectly describe their newly blocked jobs as audit-mode jobs.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
.github/workflows/claude.yml:222
- The policy above now uses
block, but this step's existing rationale still calls it anauditjob and describes the block flip as future work. That makes the documented security state contradictory; update the paragraph to describe telemetry as denied under the enforced policy.
# Same call, second vendor: turbo phones telemetry.vercel.com from the
.github/workflows/claude-implement.yml:207
- The policy above now uses
block, but this step's existing rationale still calls it anauditjob and describes the block flip as future work. That makes the documented security state contradictory; update the paragraph to describe telemetry as denied under the enforced policy.
# Same call, second vendor: turbo phones telemetry.vercel.com from the
.github/workflows/bestaxbot-reply.yml:270
- The policy above now uses
block, but this step's existing rationale still calls it anauditjob and describes the block flip as future work. That makes the documented security state contradictory; update the paragraph to describe telemetry as denied under the enforced policy.
# Same call, second vendor: turbo phones telemetry.vercel.com from the
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Preview DeploymentPreview URL: https://b9452164.bestax.pages.dev |
Copilot caught a real gap, three times over, and it is worth recording why the copied allowlist was wrong rather than just widening it. These jobs took the eight-host "Claude-session" list already enforcing on ai-scan/scan, ai-triage/triage and claude-repro/author. But none of those three runs `actions/setup-node`, so that list never needed `nodejs.org`. All six of #578's jobs DO run setup-node with `node-version: '24'`, and setup-node v7 falls back to nodejs.org/dist when the toolcache misses and the actions/node-versions lookup fails. The primary path was covered by objects.githubusercontent.com and release-assets.githubusercontent.com; the fallback was not. The measurement did not catch it because the toolcache hit on every one of the six audit runs, which is exactly the class of gap a measurement cannot close: an unexercised code path leaves no endpoint in the log. The repo-wide pattern is the check that does close it, and it is unambiguous -- every block job here that runs setup-node allows nodejs.org (auto-close-duplicates, security-txt-expiry, deploy-worker, supply-chain/consumer-sbom). The comments claimed the two githubusercontent hosts covered "the toolcache-miss path". That overstated what they deliver, which is the failure the review checklist in .github/CLAUDE.md ends on. They now name all three hosts, say which part of the path each one serves, and record why nodejs.org is the one host not in the copied set so it does not get trimmed back out later. Host resolved before listing, per rule 10.
Same gap Copilot flagged on the base branch, and the same reason: these jobs took the eight-host Claude-session list, but none of the three jobs that list came from runs `actions/setup-node`, so it never needed nodejs.org. All three jobs here do, and setup-node v7 falls back to nodejs.org/dist when the toolcache misses and the actions/node-versions lookup fails. The six audit runs could not have caught this: the toolcache hit every time, so the fallback never executed and left no endpoint in any log. A measurement only covers the paths the run took, which is worth remembering the next time a list is described as measured. Comments updated to name all three toolcache-miss hosts and what each one serves, rather than implying the two githubusercontent hosts covered the whole path.
There was a problem hiding this comment.
🟡 Changes recommended
Several workflow comments still describe audit mode or contradict the nine-endpoint allowlists.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
.github/workflows/claude.yml:78
nodejs.orgis not in the eight-host Claude-session set (as the following paragraph correctly explains), so including it among the hosts whose absence makes the measurement a strict subset of that set is internally contradictory. Distinguish the two unobserved base-set hosts from the ninth setup-node fallback.
# The eight hosts are the set already enforcing on ai-scan/scan,
# ai-triage/triage and claude-repro/author, and the measurement came out a
# strict subset of it: objects.githubusercontent.com,
# release-assets.githubusercontent.com and nodejs.org never appeared, because
# the Node 24 toolcache hit on every run. All three stay for the
.github/workflows/claude-implement.yml:56
nodejs.orgis not in the eight-host Claude-session set (as the following paragraph correctly explains), so including it among the hosts whose absence makes the measurement a strict subset of that set is internally contradictory. Distinguish the two unobserved base-set hosts from the ninth setup-node fallback.
# The eight hosts are the set already enforcing on ai-scan/scan,
# ai-triage/triage and claude-repro/author, and the measurement came out a
# strict subset of it: objects.githubusercontent.com,
# release-assets.githubusercontent.com and nodejs.org never appeared, because
# the Node 24 toolcache hit on every run. All three stay for the
.github/workflows/bestaxbot-reply.yml:89
nodejs.orgis not in the eight-host Claude-session set (as the following paragraph correctly explains), so including it among the hosts whose absence makes the measurement a strict subset of that set is internally contradictory. Distinguish the two unobserved base-set hosts from the ninth setup-node fallback.
# The eight hosts are the set already enforcing on ai-scan/scan,
# ai-triage/triage and claude-repro/author, and the measurement came out a
# strict subset of it: objects.githubusercontent.com,
# release-assets.githubusercontent.com and nodejs.org never appeared, because
# the Node 24 toolcache hit on every run. All three stay for the
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
Preview DeploymentPreview URL: https://a1df8049.bestax.pages.dev |
There was a problem hiding this comment.
Deep review — 0 blocking · 2 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Security | Enforcement is untestable pre-merge — these jobs trigger on issues/issue_comment, so the block flip's first real run is necessarily post-merge |
.github/workflows/claude.yml:108 |
| 2 | 🔵 Advisory | Robustness | @claude/@bestaxbot/claude-fix sessions that need an unlisted host (external fetch, native-dep CDN, cross-repo clone) are now refused — intended posture, reversible |
.github/workflows/claude.yml:109 |
Overall: The change is sound and unusually well-evidenced. Each new allowlist is the 8-host set already enforcing on ai-scan/scan, ai-triage/triage and claude-repro/author (verified — claude-repro/author carries exactly those eight) plus nodejs.org for the setup-node toolcache-miss path, and I confirmed there are no duplicate endpoints and the YAML is well-formed. The three new assertion steps are byte-identical to auto-close-duplicates.yml's apart from the credential named in the -e-failure message, and claude.yml's message ("claude[bot] with contents/issues/pull-requests write") matches that job's actual permissions: block. The rule 10 inventory now derives to 16 key-form / 17 plain-form block jobs, which reproduces exactly (grep -rn "^ *egress-policy: block$" -> 16, plain grep -> 17), and all 16 block jobs carry an assertion step. The riskiest part is inherent, not a defect: this is the one flip that cannot be exercised on a branch, so the human should focus on the post-merge checklist in the PR body — exercise each of the three triggers once and confirm "egress_policy":"block", Initialized, and no Reverted changes.
Residual risk:
- Silent policy-disable from an unresolvable allow-listed host (the
statsig.anthropic.com/#577 failure mode) — refuted: all nine hosts are well-known resolvable names, and the assertion's^Initializedpoll catches a revert even if one were not, since the agent reverts before writing that status. - A needed host missing -> job breaks post-merge — partially open by design: mitigated by five measured audit runs per job and graceful degradation (a blocked non-allow-listed host is refused, not a firewall revert), with a one-line policy revert as rollback.
private-user-images.githubusercontent.comis a known, documented degradation forclaude-implement(screenshot issues), deliberately omitted. - Telemetry re-reddening a block job weeks later — refuted:
telemetry.vercel.com(turbo, seen once across six runs) is denied at source viaTURBO_TELEMETRY_DISABLED: '1'placed in the session step'senv:(inherited by child pnpm/turbo), alongside the existingCLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC.
🏄 Cleanest flip I have paddled out to in a while, dude — every host on the allowlist was actually measured off a real run, not guessed, and the assertion step matches the lineup byte-for-byte. Only catch is you cannot test the wave til you are already riding it post-merge, so keep a hand on the rollback rope. Good to go.
Second finding from Copilot's review, and correct in all three files. The block flip left each session step's telemetry comment saying "On this `audit` job the point is the MEASUREMENT: audit mode exists to produce the endpoint list a block flip will use". That was true when it was written and false the moment the policy changed, which is the shape .github/CLAUDE.md's review checklist ends on: a comment that misdescribes its mechanism is worse than none, because the next reader stops checking. Rewritten for the enforced state. The denial now does two things and the comment says both: the allowlist refuses the Datadog host, and the setting stops the CLI attempting it, so a denied call stays out of the run instead of filling the log with refused traffic and inviting someone to "fix" it by allow-listing. The audit-phase rationale is kept as history rather than deleted, because it is why the measured list was clean in the first place. Also corrected the run counts: five to six, now that #593 supplied the verify measurement. Both of Copilot's findings on this PR were real, and neither was reachable from the measurement -- one was an unexercised fallback path, the other was prose.
Same defect Copilot found on #602, in the three jobs that live here. Each session step's telemetry comment still said "On this `audit` job the point is the MEASUREMENT: audit mode exists to produce the endpoint list a block flip will use" -- true when written, false as of this flip. Rewritten for the enforced state in all three, matching the base branch: the allowlist refuses the Datadog host and the setting stops the CLI attempting it, so a denied call stays out of the run rather than filling the log with refused traffic. Audit-phase rationale kept as history, since it is why the measured list was clean. Worth noting where this one came from. The loop cannot review these PRs (they touch .github/**, so its gate halts on protected-path), so the review side is Copilot and CodeRabbit plus an opt-in deep review. Copilot found two real defects here that six measured runs could not have: an unexercised setup-node fallback, and prose. Neither leaves an endpoint in a log.
Preview DeploymentPreview URL: https://21be919b.bestax.pages.dev |
There was a problem hiding this comment.
🔵 Needs a closer look
Security documentation contains contradictory allowlist descriptions and an outdated measurement status.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
.github/CLAUDE.md:488
- This inventory is already stale: #593 recorded the representative
verifyaudit run (33284876176) and closed, and the latest #578 update says all six jobs are measured. Keepingverifydocumented as unmeasured misstates the security rollout; describe all three remaining jobs as measured but still awaiting their block flip.
- **Audit, deliberately, pending a measured allowlist** — `claude-pr-loop` (`fix` and `verify`)
and `claude-review`. These run repo code with a model token; their block flip is the remainder
of the follow-up this rule owes, tracked in #578. `fix` and `review` are measured and waiting
only on their own PR; `verify` is the one job with no measurement yet, because it runs only
when a deep review files a blocking inline finding (#593).
.github/workflows/claude.yml:78
- This description now contradicts itself:
nodejs.orgis not one of the eight copied Claude-session hosts (as the next paragraph correctly says), so it cannot be part of the measured strict subset of that set. Distinguish the two unobserved hosts retained from the base list from the newly added setup-node fallback; rule 10 requires security comments to describe the mechanism exactly.
# The eight hosts are the set already enforcing on ai-scan/scan,
# ai-triage/triage and claude-repro/author, and the measurement came out a
# strict subset of it: objects.githubusercontent.com,
# release-assets.githubusercontent.com and nodejs.org never appeared, because
# the Node 24 toolcache hit on every run. All three stay for the
.github/workflows/claude-implement.yml:56
- This description now contradicts itself:
nodejs.orgis not one of the eight copied Claude-session hosts (as the next paragraph correctly says), so it cannot be part of the measured strict subset of that set. Distinguish the two unobserved hosts retained from the base list from the newly added setup-node fallback; rule 10 requires security comments to describe the mechanism exactly.
# The eight hosts are the set already enforcing on ai-scan/scan,
# ai-triage/triage and claude-repro/author, and the measurement came out a
# strict subset of it: objects.githubusercontent.com,
# release-assets.githubusercontent.com and nodejs.org never appeared, because
# the Node 24 toolcache hit on every run. All three stay for the
.github/workflows/bestaxbot-reply.yml:89
- This description now contradicts itself:
nodejs.orgis not one of the eight copied Claude-session hosts (as the next paragraph correctly says), so it cannot be part of the measured strict subset of that set. Distinguish the two unobserved hosts retained from the base list from the newly added setup-node fallback; rule 10 requires security comments to describe the mechanism exactly.
# The eight hosts are the set already enforcing on ai-scan/scan,
# ai-triage/triage and claude-repro/author, and the measurement came out a
# strict subset of it: objects.githubusercontent.com,
# release-assets.githubusercontent.com and nodejs.org never appeared, because
# the Node 24 toolcache hit on every run. All three stay for the
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
🎉 This PR is included in version 5.11.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.2.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.2.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
First half of #578. Flips
claude(claude.yml),implement(claude-implement.yml) andrespond(bestaxbot-reply.yml) fromegress-policy: audittoblock, each with the rule 10assertion step.
Why these three, separately from the other three
claude-review'sreviewandclaude-pr-loop'sfix/verifycan be exercised from a branchbefore merge —
pull_requestevents use the head's workflow, andworkflow_dispatchtakes a--ref. These three cannot: they trigger onissues/issue_comment, which always run thedefault-branch workflow, so their first enforced run is necessarily post-merge. Splitting them
out means a bad allowlist cannot land on all six at once.
The remaining three follow in a second PR.
verifyadditionally needs a measurement first — it isthe one job of the six with no audit run, because it fires only when a deep review files a blocking
inline finding (#593).
The allowlist is measured, not guessed
Each job's audit run was read out of its own
Post Harden runnerstep. That step dumps the agentlog, which records every DNS resolution and connection — so the endpoint list is a
gh run viewaway and no StepSecurity UI transcription was involved. The recipe is now written into rule 10, so
the next list does not have to rediscover it (this is also the instruction that produced
sign-sbom's still-unmeasured allowlist).claude.yml/claudeclaude-implement.yml/implementbestaxbot-reply.yml/respondAll three produced the same six application hosts —
api.anthropic.com,api.github.com,claude.ai,downloads.claude.ai,github.com,registry.npmjs.org— which is a strictsubset of the eight already enforcing on
ai-scan/scan,ai-triage/triageandclaude-repro/author. So this adopts a proven list rather than inventing one.Two things worth recording:
claudewould be materially narrower because it has noInstall dependenciesstep. It is not.
pnpm/action-setuppulls pnpm fromregistry.npmjs.orgeither way.objects.githubusercontent.comandrelease-assets.githubusercontent.comnever appeared —the Node 24 toolcache hit on every run. They stay for the toolcache-miss path, exactly as the
three existing block jobs already carry them.
Three observed hosts deliberately left out
telemetry.vercel.comis turbo's telemetry (process: turboin the log). It getsTURBO_TELEMETRY_DISABLED: '1'beside the existingCLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFICrather than a slot on the allowlist — the same call ci: enforce egress-block on the AI workflows instead of declaring it #577 made for the Datadog host. It showed up
in only one of the five audit runs because turbo samples, which is precisely the intermittent
that would redden a block job weeks from now. Verified against turbo 2.10.7 locally:
TURBO_TELEMETRY_DISABLED=1 turbo telemetry statusreportsDisabled.productionresultssa3/6/7/9/11allappeared across five runs. A cache miss is accepted; it degrades to a
registry.npmjs.orgrefetch. This matches the existing deliberate omission in
deploy-worker.ymlandsupply-chain.yml.private-user-images.githubusercontent.comisimplementfetching an image embedded in anissue body. Left out: the cost is that a
claude-fixissue carrying a screenshot degrades, andthe reason is that on a public repo it is the one candidate host serving attacker-supplied bytes
into a job holding bestaxbot's PAT. Revisit if a run actually needs it.
Every host was resolved before being listed. An unresolvable entry is not inert — the agent reverts
the firewall while the pre-step still exits 0, which is how
statsig.anthropic.comsilentlydisabled the policy on #577's first run.
Assertion steps
Byte-identical to
auto-close-duplicates.yml's apart from the credential each names — verifiedprogrammatically, not by eye. So the three load-bearing shapes are preserved rather than
hand-rewritten: the
[ -e /home/agent/agent.json ]check first, the 30×1s poll, and^Initializedrather than
-qx.Docs
(13 → 16 key-form, 14 → 17 plain-form, both checked with the greps the rule prescribes).
and which still have that leg in name only.
that must never be allow-listed, and the "one run is not a measurement" caveat.
Verification
Local:
prettier --check,check:conformance(all 15 green), both rule 1 pin greps, and ajs-yaml parse of every touched workflow confirming policy/endpoint-count/assertion-adjacency.
Post-merge, before this is considered done — each job exercised once (an
@claudecomment, aclaude-fixlabel, an@bestaxbotmention), checking that the assertion step is green, the configreads
"egress_policy":"block", the agent log saysInitializedwith noReverted changesortimed out, and the session actually completed its work. Rollback is a one-line policy revert.Refs #578