security: scope reusable-workflow secrets (least privilege) over secrets: inherit #9
Reference in New Issue
Block a user
Delete Branch "sec/scope-secrets"
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?
Fixes the real finding the swarm raised on the mort/executus rollout PRs:
secrets: inheritover-shares — it forwards every caller secret (registry/Komodo/Discord/DB creds) to the reusable review workflow, which only needs the review ones.Change
review-reusable.yml: declareon.workflow_call.secrets:(allrequired: false) so a caller can forward only what the reviewer uses.adversarial-review.yml(gadfly's own caller) +examples/reusable.yml: replacesecrets: inheritwith an explicit forward of justOLLAMA_CLOUD_API_KEY/CLAUDE_CODE_OAUTH_TOKEN/ findings tokens.GITEA_TOKENstays automatic.@<sha>instead of@main(the second finding).Why this PR self-validates
gadfly's own caller uses a local reusable ref (
./.gitea/workflows/review-reusable.yml), so this PR's own review run exercises the new explicit-secrets contract end-to-end on the act_runner. If the cloud + Claude Code models authenticate and the comment posts, the path is proven — then I'll adopt it on mort #1338 / executus #13 and pin them to this repo's merge SHA.Advisory-only invariant unchanged.
🤖 Generated with Claude Code
🪰 Gadfly — live review status
9/9 reviewers finished · updated 2026-06-28 01:04:38Z
claude-code/opus· claude-code — ✅ doneclaude-code/opus:max· claude-code — ✅ doneclaude-code/sonnet· claude-code — ✅ donedeepseek-v4-pro:cloud· ollama-cloud — ✅ doneglm-5.1:cloud· ollama-cloud — ✅ doneglm-5.2:cloud· ollama-cloud — ✅ doneminimax-m3:cloud· ollama-cloud — ✅ donenemotron-3-super:cloud· ollama-cloud — ✅ doneqwen3-coder:480b-cloud· ollama-cloud — ✅ doneLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
🪰 Gadfly review —
glm-5.1:cloud(ollama-cloud)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:95— stale comment says "via secrets: inherit": The env-block comment reads# --- provider auth (via secrets: inherit; empty if consumer unset) -, but the PR replacedsecrets: inheritwith explicit per-secret forwarding. The comment is now inaccurate and could mislead a future editor into reverting tosecrets: inherit. It should be updated to reflect the explicit-secret-forwarding contract.examples/reusable.yml:49— example still pins@main: Lines 47–48 explicitly warn: "Pin @ to an immutable Gadfly commit (replace @main below) so a push to gadfly can't silently change the code that runs with your forwarded secrets." Yet line 49 still readsuses: steve/gadfly/.gitea/workflows/review-reusable.yml@main. The insecure default is what copy-paste users will deploy, undermining the supply-chain-integrity objective.🎯 Correctness — Minor issues
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:95— Stale comment still reads# --- provider auth (via secrets: inherit; empty if consumer unset) -but the entire point of this PR is to move away fromsecrets: inherittoward explicitworkflow_call.secretsforwarding. The comment is misleading to future maintainers who may read it as guidance rather than the surrounding code. Should be updated to reflect the declared-secret model (e.g.,via declared workflow_call secrets). Impact: cosmetic/documentation, no functional effect.GITEA_TOKENswitch fromsecrets.GITEA_TOKEN→github.token(line 87) — Confirmed correct.github.tokenis a context variable always available to both caller and callee jobs, whereassecrets.GITEA_TOKENwould be empty under explicit forwarding sinceGITEA_TOKENis not declared inworkflow_call.secrets. No issue here.All
secrets.*env references in the reusable match aworkflow_call.secretsdeclaration — Confirmed by cross-referencing lines 96–109 (env block) against lines 56–65 (declarations). Every secret the reusable uses is declared, and every declared secret is consumed. No mismatch found.🧯 Error handling & edge cases — Minor issues
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:95— Stale comment referencingsecrets: inherit: The comment# --- provider auth (via secrets: inherit; empty if consumer unset) -on line 95 was not updated in this PR despite the change away fromsecrets: inheritto explicitworkflow_call.secretsforwarding. While this doesn't cause a runtime error, it's a misleading comment in a security-sensitive section. A future maintainer reading this could reasonably believe secrets are still inherited, and make incorrect decisions about what the reusable workflow can access. The comment should be updated to reflect the explicit-secret-forwarding model (e.g.,# --- provider auth (via declared workflow_call secrets; empty if caller doesn't forward) -).Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 29s
🪰 Gadfly review —
claude-code/opus(claude-code)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
I've verified the change against the actual workflow code and the consuming scripts. Here's my security-lens review.
VERDICT: No material issues found
This PR is itself a security hardening change (replacing
secrets: inheritwith least-privilege explicit forwarding). I traced the token and secret flow to confirm it introduces no new exposure or authz gap:GITEA_TOKEN: ${{ github.token }}swap (review-reusable.yml:87) — verified safe.github.tokenis the auto-provisioned job token, present regardless of secret forwarding. Its capabilities are bounded by the job'spermissions:block (review-reusable.yml:69-72:contents: read,issues: write,pull-requests: write). If anything this is tighter least-privilege than the oldsecrets.GITEA_TOKENpath. The token is consumed exactly as before — clone URL (entrypoint.sh:126), collaborator authz gate (entrypoint.sh:81), and comment POST/PATCH (run.sh,status-board.sh). No security control lost the credential it depends on.Declared-vs-referenced secrets are consistent — all 10 secrets referenced in the
env:block (review-reusable.yml:96-109) are declared underon.workflow_call.secrets:(lines 56-65). No env var silently resolves to empty due to an undeclared secret, and no secret is declared-but-leaked.GADFLY_ENDPOINT_M1/M5(which can carry an embedded provider key in theirprovider|url|keyDSN) are correctly classified as secrets, not inputs.Reduced secret blast radius — the core change is the security win: the reviewer image no longer receives the caller's registry/deploy/DB creds it never uses. The
@<sha>pinning guidance added to the docs further mitigates the supply-chain risk of@mainsilently changing what runs with forwarded secrets. Both are correct hardening.No new tainted-input path — the change touches only secret/token plumbing; untrusted fields like
COMMENT_BODY/PR_BRANCHare unchanged by this diff (pre-existing, out of this PR's scope).All findings above were verified by reading
review-reusable.ymlin full and grepping everyGITEA_TOKEN/github.tokenconsumer acrossentrypoint.sh,scripts/run.sh, andscripts/status-board.sh. Nothing in the security lane is materially wrong.🎯 Correctness — No material issues found
I've verified the change against the actual checked-out code. This PR is config/YAML only; through the Correctness lens I checked the behavioral substance rather than the prose.
VERDICT: No material issues found
Findings (all verified by reading the files):
github.tokensubstitution for the API token is consistent and complete.review-reusable.yml:87now setsGITEA_TOKEN: ${{ github.token }}. I confirmed every downstream consumer reads the env varGITEA_TOKEN(not a secret):entrypoint.sh:61(:?guard),:65/:81/:108/:126(clone URL + API), andscripts/run.sh:45/:80/:89/:92/:102-103. So the rename of the source (secrets.GITEA_TOKEN→github.token) is purely on the right-hand side; the variable name the scripts depend on is unchanged. No broken reference. The reusable job declaresissues: write/pull-requests: write(:69-72), which is whatgithub.tokenneeds to upsert the comment — correct.Caller forwards only declared secrets — no
workflow_callvalidation error.adversarial-review.ymlforwardsOLLAMA_CLOUD_API_KEY,CLAUDE_CODE_OAUTH_TOKEN,GADFLY_FINDINGS_URL,GADFLY_FINDINGS_TOKEN. All four are declared inreview-reusable.yml:55-65. Passing an undeclared secret in asecrets:map is a hard error in Actions; here every forwarded name is declared, so this is safe.The forwarded set actually covers the models this caller runs. The
models:list is all*:cloud(→OLLAMA_CLOUD_API_KEY) plusclaude-code/sonnet|opus|opus:max(→CLAUDE_CODE_OAUTH_TOKEN). Both keys are forwarded, so droppingsecrets: inheritdoes not starve any configured model of its credential. The omitted keys (OPENAI/ANTHROPIC/GOOGLE/GADFLY_API_KEY,GADFLY_ENDPOINT_M1/M5) are unused by this model list, so their absence is correct, not a regression.github.tokenavailability — the one runtime assumption I could not statically verify. The whole change hinges ongithub.tokenbeing populated inside aworkflow_calljob on Gitea's act_runner (andsecrets.GITEA_TOKENbeing empty under explicit forwarding, as the comment at:83-86claims). I can't confirm act_runner's context behavior from the repo. Ifgithub.tokenwere empty,entrypoint.sh:61's:?guard fails fast (advisory-only invariant preserved — it won't silently mis-post). The PR's self-validation design (local./ref exercising this path) is the right way to confirm it; flagging only as the assumption to watch, not as a defect.Nothing in the Correctness lane is materially wrong.
Outside my lens: the step comment at
review-reusable.yml:95("via secrets: inherit") is now stale post-change — a docs/maintainability nit for that reviewer.🧯 Error handling & edge cases — No material issues found
I've verified the consumers of the changed values. Here's my review through the error-handling & edge-cases lens.
VERDICT: No material issues found
This PR is almost entirely declarative YAML and docs (secret declarations, an env-source swap, comment/README updates). I checked the unhappy paths it touches against the actual scripts that consume these values:
GITEA_TOKENfromsecrets.GITEA_TOKENtogithub.token(review-reusable.yml:87). Bothentrypoint.sh:61andscripts/run.sh:45guard with: "${GITEA_TOKEN:?GITEA_TOKEN required}", so an empty token fails fast with a clear message rather than silently making unauthenticated API calls. The diff doesn't weaken this guard — it only changes the value's source. (Verified by reading both scripts.)run.sh(|| echo '[]',|| true) andentrypoint.shis untouched.workflow_call.secrets(lines 56–65) exactly match the env keys mapped at lines 96–109, so no consumed-but-undeclared secret silently resolves empty beyond what was already possible. Unset optional secrets resolving to""is the pre-existing, guarded behavior for the provider keys (each provider path tolerates its own key being empty). (Verified by cross-referencing the two blocks.)One edge case worth the author's attention, but it is primarily a correctness question outside this lens and I could not verify it from the repo: the whole change rests on
github.tokenbeing auto-populated in this act_runner'sworkflow_callcontext for all trigger event types (PR,issue_comment, manual dispatch). If it is ever empty for some event,entrypoint.sh:61hard-exits before posting — fail-fast, not a silent error, so even the worst case is safe. The PR's self-validation run exercises only theissue_comment/PR path, so confirm the other triggers too. No error-handling defect here regardless of outcome.Nothing in my lane is materially wrong.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 14s
🪰 Gadfly review —
glm-5.2:cloud(ollama-cloud)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
Verdict: Minor issues
Through the security (least-privilege / secret-scoping) lens, the core change is sound: replacing
secrets: inheritwith an explicit declaredworkflow_call.secrets:block and forwarding only what the reviewer uses is the correct direction. Verified againstreview-reusable.yml:55-65(declared secrets, allrequired: false) and the two callers (adversarial-review.yml:45-49,examples/reusable.yml:53-57). TheGITEA_TOKEN: ${{ github.token }}switch atreview-reusable.yml:87is also correct — the auto job token comes from the context, not a forwarded secret, so it survives the move offsecrets: inherit.Findings:
review-reusable.yml:95— stale/misleading comment "via secrets: inherit". Confirmed in source: the env block readingsecrets.OLLAMA_CLOUD_API_KEYetc. is still labeled# --- provider auth (via secrets: inherit; empty if consumer unset) -. After this PR those values come from the declaredworkflow_call.secretsunder explicit forwarding, notsecrets: inherit. Not a live vulnerability (the values are still scoped correctly), but the comment mis-states the trust boundary and could mislead a future maintainer into re-broadening the contract. Suggested fix: rewrite to "via declared workflow_call secrets; empty if caller doesn't forward the key."examples/reusable.yml:49—@mainstill used despite the security warning it ships. Confirmed: the PR adds a strong comment ("pin@<sha>… so a push to gadfly can't change what runs with your forwarded secrets") at lines 47-48 directly aboveuses: steve/gadfly/.gitea/workflows/review-reusable.yml@mainat line 49. Shipping the example pinned to@mainmeans a copy-paste consumer silently runs attacker-controllable code (a future push to this repo'sreview-reusable.yml) with the consumer's forwarded provider credentials in env. This is the exact supply-chain secret-exposure risk the PR's own prose warns about; the example is the template real consumers will copy verbatim. Suggested fix: either use a real immutable SHA in the example (even if it needs bumping per release) or make the@maina clearly-failing placeholder like@REPLACE-WITH-IMMUTABLE-SHAso a copied stub can't run as-is.CLAUDE.md:50— documentation still instructssecrets: inherit. Confirmed in source (line 50):an ~8-line caller +secrets: inherit(Phase 4).Not touched by the diff, but it's the in-repo design doc still describing the subscribe path that way. Minor (docs drift), but it undercuts the least-privilege guidance for anyone reading CLAUDE.md. Suggested fix: update to reference the explicit-secret forwarding form.No blocking secret-leak issues found in my lane: the declared-secret set matches exactly the env vars consumed at
review-reusable.yml:96-109(no secret is read that isn't declared, so nothing silently falls back to broad inheritance), andgithub.tokencorrectly replaces the previously-forwardedsecrets.GITEA_TOKEN.🎯 Correctness — Minor issues
VERDICT: Minor issues found
review-reusable.yml:95— Stale inline comment still reads# --- provider auth (via secrets: inherit; empty if consumer unset) -directly above the provider-auth env block. The reusable workflow no longer relies onsecrets: inherit; it now uses explicitly declaredworkflow_call.secretsthat callers forward selectively. The header comment block (lines 17–22, 48–54) was updated to describe the new explicit-forwarding contract, but this inline header at line 95 was left describing the deprecatedsecrets: inheritpath. Minor doc drift that could mislead a future editor into re-addingsecrets: inherit. Verified by readingreview-reusable.yml:95against the surrounding declared-secrets block (:55–65) and the updated header comments.The "Not issues" observations in the draft hold up: the
adversarial-review.ymlcaller forwards exactly the secrets its model list consumes (OLLAMA_CLOUD_API_KEYfor the:cloudmodels,CLAUDE_CODE_OAUTH_TOKENfor theclaude-code/*models, plus the two findings-telemetry secrets); the omitted provider keys resolve to empty in the reusable env and are harmless. The declaredsecrets:block with allrequired: falseis consistent with callers forwarding a subset, andexamples/reusable.ymlforwarding onlyOLLAMA_CLOUD_API_KEYmatches its documented Ollama-Cloud-default usage.🧯 Error handling & edge cases — No material issues found
Verdict: No material issues found
Through the error-handling / edge-case lens, the diff is config-only YAML; it adds no new code paths that swallow errors or skip cleanup. I verified the one unhappy path it touches — an empty
GITEA_TOKEN:entrypoint.sh:61guards with: "${GITEA_TOKEN:?GITEA_TOKEN required}", and the env is reused for cloning (entrypoint.sh:126) and API calls (entrypoint.sh:65,81,108). The diff switches the source fromsecrets.GITEA_TOKENtogithub.token(review-reusable.yml:87). If that context value were ever empty, the run fails loudly at line 61 rather than proceeding with a blank credential — i.e. the edge case is handled, not swallowed. That is the correct behavior for an unhappy path.The newly declared
workflow_call.secrets:are allrequired: false(review-reusable.yml:56-65), matching the "optional, resolves to empty, harmless" contract the comments describe. The caller inadversarial-review.yml:45-49forwards only names that are declared in that list (OLLAMA_CLOUD_API_KEY, CLAUDE_CODE_OAUTH_TOKEN, GADFLY_FINDINGS_URL, GADFLY_FINDINGS_TOKEN) — so no "undeclared secret" hard error; theexamples/reusable.yml:54stub likewise forwards onlyOLLAMA_CLOUD_API_KEY. Verified against the declaration block.Minor (not blocking, arguably outside this lens — comment accuracy, not a runtime edge):
review-reusable.yml:95still reads# --- provider auth (via secrets: inherit; empty if consumer unset) -, but this PR moves away fromsecrets: inheritto declared/forwarded secrets. The stale wording could mislead a future maintainer into re-addingsecrets: inherit. Suggest updating to "via declared workflow_call secrets; empty if caller doesn't forward it."Unverified (not reported as a finding): whether
github.tokenis reliably populated by this Gitea/act_runner version in aworkflow_call— but if it were empty, the:?guard above makes the failure loud rather than silent, so it stays out of "swallowed error" territory.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 35s
🪰 Gadfly review —
claude-code/sonnet(claude-code)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
Verdict: No material issues found
I verified the actual checked-out state of all three workflow files, and the scripts that consume
GITEA_TOKEN.GITEA_TOKEN: ${{ github.token }}— confirmed against the full file.run.shandstatus-board.shboth requireGITEA_TOKENfor API calls (comment POST/PATCH, PR diff fetch). Switching fromsecrets.GITEA_TOKEN(a PAT that would now be absent under explicit forwarding) togithub.token(the auto job-scoped token) is correct and strictly more secure: the auto-token is ephemeral, rotates per-job, and is constrained to thepermissions:block (issues: write,pull-requests: write). In Gitea'sworkflow_callcontextgithub.tokenis scoped to the calling repository, so comment posting still lands on the caller's repo. Verified by readingscripts/run.shandscripts/status-board.sh— they expect an env var namedGITEA_TOKEN; the name mapping is correct.Secret declarations in
workflow_call.secrets:—GADFLY_ENDPOINT_M1/GADFLY_ENDPOINT_M5are correctly typed as secrets (they can embed API keys per the DSN format); verified they appear in thesecrets:block, notinputs:. All ten secrets arerequired: false, so an unset secret resolves to empty without breaking the job.GADFLY_FINDINGS_URL/GADFLY_FINDINGS_TOKENforwarded inadversarial-review.ymland declared in the reusable — confirmed both sides match.Supply-chain note in examples/reusable.yml still uses
@mainas theuses:ref — but this is an intentional template placeholder; the surrounding comment explicitly directs users to replace it with an immutable SHA. This is documentation intent, not a code defect.The change is a net improvement: it eliminates the over-broad
secrets: inheritsurface, replaces a long-lived PAT dependency with the ephemeral auto-token, and the declared-secrets approach limits cross-contamination of unrelated caller credentials into the review workflow.🎯 Correctness — Minor issues
The comment at line 95 is confirmed as stated in the draft. The finding is accurate.
Minor issues
The core change is semantically correct. In Gitea Actions (and GitHub Actions),
${{ github.token }}in aworkflow_callreusable workflow is always populated by the runner as the auto job token — it is part of thegithubcontext, not thesecretscontext, so it is present regardless ofsecrets: inherit. The old${{ secrets.GITEA_TOKEN }}would be empty under explicit secret forwarding (since GITEA_TOKEN is not in the forwarded list), triggering the: "${GITEA_TOKEN:?GITEA_TOKEN required}"guard at bothentrypoint.sh:61andrun.sh:45. The switch togithub.tokenis the correct fix.Permission scopes are coherent: both the caller (
adversarial-review.yml:22-25) and the example consumer (examples/reusable.yml:31-34) declareissues: writeandpull-requests: write, matching what the reusable workflow declares atreview-reusable.yml:69-72. The auto token will carry those scopes into the container steps that POST comments and reactions.The declared
secrets:block (lines 55-65) and theenv:mappings (lines 96-109) are consistent — every declared secret is mapped, every mapped secret is declared. Verified by reading both sections.review-reusable.yml:95— stale comment contradicts the PR's change. The comment reads:secrets: inheritto explicit forwarding. This comment now misleads a reader into thinkingsecrets: inheritis still the mechanism. It should say something like "via explicit workflow_call secret forwarding; resolves to empty if caller doesn't forward or consumer hasn't set the secret". Verified by reading the file — the comment is at line 95 of the checked-outreview-reusable.yml.🧯 Error handling & edge cases — No material issues found
All 10 declared secrets are properly wired. My investigation is complete.
Verdict: No material issues found
Through the error-handling & edge-cases lens, this change is clean:
GITEA_TOKEN: ${{ github.token }}switch is actually more robust. Under the oldsecrets.GITEA_TOKENwith explicit forwarding, that secret resolves to empty — which hits the${GITEA_TOKEN:?GITEA_TOKEN required}hard-exit inentrypoint.shandrun.sh.github.tokenis always populated in aworkflow_callcontext (it carries the caller's auto-job token), so the mandatory-token guard can never trip on a missing value anymore. (Verified:entrypoint.shandrun.shboth use${GITEA_TOKEN:?…}as an immediate-exit guard.)All 10 declared secrets have corresponding env-var mappings. Every
on.workflow_call.secretsentry is referenced in the job'senv:block. None are declared but silently dropped. (Verified: lines 96–109 ofreview-reusable.yml.)Empty-secret resolution is already handled gracefully. Provider keys are all
required: false;run.shalready checks for a missing Ollama Cloud key (and similar) before running, emitting a warning comment rather than crashing. Unset secrets become empty env vars — which is the existing "this provider isn't configured" path. Nothing new is broken here.No new unhandled error paths. The diff touches only YAML wiring and doc comments — no new logic in
entrypoint.sh,run.sh, or the Go binary. All unhappy paths (empty token, missing API key, curl failure) were pre-existing and remain unchanged.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 7m 54s
🪰 Gadfly review —
minimax-m3:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
I now have a complete picture for the security lens. Time to write up the review.
VERDICT: No material issues found (in my security lens)
I traced the change end-to-end against the actual code:
Verified by reading:
.gitea/workflows/review-reusable.yml— the newworkflow_call.secrets:block (lines 55-65) is properly declared; the switch fromsecrets.GITEA_TOKENtogithub.token(line 87) correctly reflects Gitea's auto job token (verified bygrep: no other reference to a user-definedGITEA_TOKENsecret exists in the repo, only the auto token)..gitea/workflows/adversarial-review.yml(lines 42-49) — correctly forwards exactly the four secrets the dogfood swarm uses, and switches fromsecrets: inheritto explicit forwarding.examples/reusable.yml(lines 49-57) — same explicit-forward pattern;CLAUDE_CODE_OAUTH_TOKENand the findings pair are commented out with a hint that they should be uncommented only if used.entrypoint.sh:60-65—: "${GITEA_TOKEN:?…}"and theAuthorization: token ${GITEA_TOKEN}curl path; the env is sourced from the workflow env block, whichgithub.tokenpopulates even under explicit secret forwarding.cmd/gadfly/engine.go:216-241(claudeEnv) andcmd/gadfly/engine_test.go:120-150— confirmed the claude-code subprocess getsCLAUDE_CODE_OAUTH_TOKENvia the engine's keep-list (test asserts it's kept;GITEA_TOKENand provider keys are stripped) andTestClaudeEnvFiltersalready pins that behavior. SoCLAUDE_CODE_OAUTH_TOKENis needed by the reusable even though it isn't used by the parent container, and the forward is correct.grepforsecrets: inheritacross the repo — only the new doc-comments and CHANGELOG-style historical mentions remain; no live workflow still uses it.grepforsecrets.GITEA_TOKENinexamples/— the non-reusable examples (adversarial-review.yml,claude-code.yml,endpoint-aliases.yml,local-ollama.yml,openai-compatible.yml) still passsecrets.GITEA_TOKEN, but that's a Gitea-Actions built-in token (not a user-defined secret), sosecrets.GITEA_TOKENresolves to the auto token there too — harmless, and out of scope for this PR (those examples aren't callers of the reusable).What the change gets right, security-wise:
workflow_call.secretsarerequired: false, so callers can forward a strict subset.github.tokenswitch is correct and avoids a regression that would have emptiedGITEA_TOKENunder the new explicit-forward contract (the comment at lines 83-87 documents this nicely).@<sha>not@main) is good defense against silent upstream code drift that would run with forwarded secrets.One small, non-blocking observation (advisory, verified):
examples/reusable.yml:55-57showsCLAUDE_CODE_OAUTH_TOKENand the findings pair as commented-out examples under a section that the comment block (lines 9-14) describes as the secrets the reviewer needs. A copy-paste reader who follows the comment block literally won't enableCLAUDE_CODE_OAUTH_TOKEN, then if theirGADFLY_MODELSincludesclaude-code/<model>they'll get an auth failure at runtime. The README at lines 107-109 already says the default path needs this token, and the inlineexamples/claude-code.ymlshows the standalone path — but the slim reusable example doesn't make the "if your models include claude-code, uncomment this" condition obvious. This is a docs clarity nit rather than a security bug; happy to leave it as-is given the PR's scope.No blocking or material issues in this lens.
🎯 Correctness — No material issues found
VERDICT: No material issues found
Through the Correctness lens, the change is a clean semantic refactor:
.gitea/workflows/review-reusable.yml:55-65—on.workflow_call.secrets:correctly declares each secret asrequired: false, so an unforwarded secret resolves to empty (verified the env block at lines 96-109 expects that and the image treats empty as "use built-in default"). Verified valid YAML structure..gitea/workflows/review-reusable.yml:87— switchingGITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}→${{ github.token }}is the right substitution: in aworkflow_callcontext the auto job token lives ongithub.token, not as a forwarded secret. The inline comment at lines 83-86 explains exactly this, and verifiedentrypoint.sh:61still requiresGITEA_TOKEN(the env var), so the value just has to come from a different source. Semantics preserved..gitea/workflows/adversarial-review.yml:42-49— localuses: ./.gitea/workflows/review-reusable.ymlstill goes through theworkflow_callcontract on this Gitea (verified by the same env-block expectations), so explicit forwarding is required and now present. The 4 forwarded secrets match the swarm config (cloud + Claude Code + findings); other provider keys are intentionally not forwarded because the model list (minimax-m3:cloud,glm-5.*:cloud,…,qwen3-coder:480b-cloud,claude-code/*) doesn't reference OpenAI/Anthropic/Google/foreman endpoints. No correctness regression.examples/reusable.yml:53-57— the explicitsecrets:mapping nests correctly underjobs.review; commented-out lines inside a YAML mapping are valid;examples/reusable.yml:49still pins@mainbut the comment above (lines 47-48) explicitly tells the consumer to replace it with@<sha>— that's documentation, not a logic defect.The non-reusable stubs (
examples/adversarial-review.yml,local-ollama.yml, etc.) keep${{ secrets.GITEA_TOKEN }}— correct, since those run as their own jobs (notworkflow_call) and inherit the auto-injected runner secret normally. Not in this PR's scope but verified consistent.No off-by-one, no wrong constant/threshold, no broken formula, no semantic regression in the secret-scoping contract.
🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
Through the error-handling & edge-case lens, this PR is clean.
What I verified by reading the files:
.gitea/workflows/review-reusable.yml:87(GITEA_TOKEN: ${{ github.token }}) — the most behavior-changing edit. Under explicit secret forwarding the oldsecrets.GITEA_TOKENwould resolve to empty, so the diff moves togithub.token(the auto job token). Thepermissions:block at lines 70-72 already declaresissues: write+pull-requests: write, so the token has the scopes needed to post the review comment and clone the PR. The explanatory comment at lines 83-86 makes the rationale grep-able. No error swallow, no silent empty-token fallback.secrets: inherit→ explicit forwarding — every declared secret isrequired: false, and the reusedcmd/gadfly/emit.go:97-100already guards withif base == "" { return }so an unsetGADFLY_FINDINGS_URL(the most failure-prone env) cleanly disables telemetry instead of failing. The same "empty ⇒ harmless" pattern is what every other consumer key relies on, and there's no new code path that breaks when a forwarded secret is blank.github.tokenis the right source. A consumer still onsecrets: inheritwould still work becausegithub.tokenis context-provided, not a declared workflow_call secret. No off-by-one / boundary risk introduced.timeout_minutesinput (default 45, caller passes 90) — declared as{ type: number, default: 45 }at line 47, wired totimeout-minutes: ${{ inputs.timeout_minutes }}at line 77, andadversarial-review.yml:58passestimeout_minutes: 90. Type/range handling unchanged.examples/reusable.yml— the commented-outCLAUDE_CODE_OAUTH_TOKEN/ findings block (lines 55-57) is documented inline as opt-in, so a copy-paste consumer that forgets to enable them just runs a smaller config rather than hitting a missing-secret panic.emit.go's "never return, never exit, never touch stdout" contract at lines 92-95 is unchanged and not regressed by the env plumbing change in the workflow.entrypoint.shandcmd/gadflyare untouched.Outside my lens: the diff is correctly a security scoping change; correctness and security are covered by other reviewers.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 2m 1s
🪰 Gadfly review —
deepseek-v4-pro:cloud(ollama-cloud)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
Both findings confirmed against the actual code. Outputting the corrected review.
Verdict: Minor issues
review-reusable.yml:95— stale comment still says "via secrets: inherit". The env-block comment# --- provider auth (via secrets: inherit; empty if consumer unset) -is now misleading. The mechanism changed fromsecrets: inheritpassthrough to explicitworkflow_call.secretsdeclarations. A future maintainer reading this could be confused about how secrets actually flow into the container. Fix: s/via secrets: inherit/via workflow_call secrets/ or similar.examples/reusable.yml:49— example still references@main(mutable ref). Theuses:line says@maindespite the comment directly above it telling users to "Pin @<sha> to an immutable Gadfly commit (replace @main below)." If a consumer copies this file verbatim without changing the ref, a push tosteve/gadflymain can silently change the code that runs with their forwarded secrets — a supply-chain risk. The comment mitigates this for attentive users, but "secure by default" would mean the example itself uses a pinned SHA (or at least@<tag>).🎯 Correctness — Minor issues
Verdict: Minor issues
review-reusable.yml:87—github.tokenmay not be populated in a Giteaworkflow_callreusable workflow (unverified). The old code relied onsecrets.GITEA_TOKENarriving viasecrets: inherit. The new code switches to${{ github.token }}, asserting it's "the auto job token from the github CONTEXT" and "present even withoutsecrets: inherit." In GitHub Actions this is true —github.tokenis the caller's token and is available in reusable workflows. But Gitea's act_runner (nektos/act-based) is not GitHub Actions; whether it populatesgithub.tokeninside aworkflow_callcallee is a behavioral dependency this repo has never exercised before (confirmed via grep: no other file in this repo referencesgithub.token). If act_runner leaves it empty, the review job silently loses the ability to post its comment — the PR's own self-review would fail to appear, which is the intended validation, but the code as committed bakes in an unverified assumption.Suggested fix: Either (a) verify
github.tokenworks in a Gitea reusable workflow before merging, or (b) declareGITEA_TOKENas an additionalworkflow_callsecret (required: false) and have callers forward${{ secrets.GITEA_TOKEN }}alongside the other secrets — this keeps the auto token explicit and avoids depending on context semantics that may differ from GitHub Actions.review-reusable.yml:95— stale comment still sayssecrets: inherit. The env-block comment reads# --- provider auth (via secrets: inherit; empty if consumer unset) -. The secrets now arrive through the explicitly declaredworkflow_call.secretsblock, notsecrets: inherit. The comment is misleading for anyone reading the file to understand the data flow. (Verified by reading the file at line 95.)Suggested fix: Change to
# --- provider auth (via workflow_call secrets; empty if consumer unset) -.🧯 Error handling & edge cases — No material issues found
No material issues found
The diff is clean through the error-handling lens. Here's what I verified:
github.tokensubstitution (review-reusable.yml:87): The change fromsecrets.GITEA_TOKENtogithub.tokenis necessary because under explicit secret forwarding,secrets.GITEA_TOKENwould be empty (the auto job token isn't a forwardedworkflow_callsecret). Ifgithub.tokenwere somehow unavailable in the reusable workflow context,entrypoint.sh:61would fail immediately withGITEA_TOKEN required— a safe, loud failure, not a silent one. Verified by reading both files end-to-end.All secrets
required: false(review-reusable.yml:55-65): Unforwarded secrets resolve to empty strings. The entrypoint (entrypoint.sh:139-142) defaults findings vars to empty with${VAR:-}, and the Go binary gates on non-empty before using any provider key. No panic or crash path from a missing secret. Verified by reading the full entrypoint and the env plumbing in the reusable workflow.Caller forwards only needed secrets (
adversarial-review.yml:45-49): The four forwarded secrets match what the swarm actually uses (Ollama Cloud + Claude Code + findings telemetry). The five undeclared provider secrets (OPENAI_API_KEY,ANTHROPIC_API_KEY,GOOGLE_API_KEY,GADFLY_API_KEY,GADFLY_ENDPOINT_M1/M5) correctly resolve to empty — harmless for this caller. Verified by cross-referencing the caller'smodels:line against the reusable workflow's env map.examples/reusable.ymlforwards onlyOLLAMA_CLOUD_API_KEYwith the rest commented out — appropriate for the default Ollama Cloud use case. No error-handling gap.No missing cleanup/rollback: The diff doesn't introduce any resource acquisition that would need a
deferor rollback. The entrypoint's existingrm -rf "$REPO_DIR"and status-board signal-via-touchfile pattern are unchanged.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 1s
🪰 Gadfly review —
nemotron-3-super:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
No material issues found
The PR successfully implements least-privilege secret forwarding for reusable workflows, replacing
secrets: inheritwith explicit secret forwarding in all relevant workflows. Key security improvements verified:.gitea/workflows/review-reusable.ymlnow declares all possible secrets withrequired: false, allowing callers to forward only needed secrets..gitea/workflows/adversarial-review.yml(gadfly's own workflow) now forwards onlyOLLAMA_CLOUD_API_KEY,CLAUDE_CODE_OAUTH_TOKEN,GADFLY_FINDINGS_URL, andGADFLY_FINDINGS_TOKENinstead ofsecrets: inherit, preventing unnecessary exposure of other secrets (e.g., registry/deploy/DB credentials).GITEA_TOKENfrom${{ secrets.GITEA_TOKEN }}to${{ github.token }}since the automatic token isn't a forwarded secret.@mainas starter kits, with guidance to pin post-validation).All changes align with the security goal of minimizing secret exposure in reusable workflows. No unverified or insecure patterns were found in the reviewed files.
Note: The PR author notes they'll pin consumers to an immutable SHA after validating this PR (as mentioned in the description). While the examples still reference
@main, this is advisory only and the local reusable workflow used by gadfly itself is not vulnerable to supply-chain tampering. No blocking security issues were identified.🎯 Correctness — No material issues found
No material issues found
🧯 Error handling & edge cases — No material issues found
No material issues found
The changes improve security and error handling by:
Fixing GITEA_TOKEN handling in review-reusable.yml: Changed from
${{ secrets.GITEA_TOKEN }}to${{ github.token }}with explanatory comments. This prevents authentication failures when using explicit secret forwarding (since the automatic GitHub token isn't passed via secrets mechanism). Verified by reading the updated lines 83-87 in .gitea/workflows/review-reusable.yml.Least-privilege secret forwarding: Both adversarial-review.yml and examples/reusable.yml now explicitly forward only required secrets instead of using
secrets: inherit, reducing exposure of unrelated secrets. Verified by reading the changes in those files.Documentation updates clarify the security implications and pinning recommendations. Verified in README.md and examples/README.md.
No error handling issues, edge cases, or missing cleanup were identified in the changes. The self-validating nature of the PR (using the updated workflow in adversarial-review.yml) further confirms the changes work correctly.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 2m 29s
🪰 Gadfly review —
qwen3-coder:480b-cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
VERDICT: No material issues found
The changes in this pull request improve the security posture by implementing the principle of least privilege for secret handling in reusable workflows. Here are the key improvements:
The
review-reusable.ymlworkflow now declares explicitsecretsinworkflow_callwith all secrets marked asrequired: false, allowing callers to forward only the secrets the reviewer actually needs.The calling workflows (
.gitea/workflows/adversarial-review.ymlandexamples/reusable.yml) have been updated to explicitly list the secrets they pass, rather than usingsecrets: inherit. This prevents unnecessary exposure of secrets like registry credentials, deployment keys, and database credentials to the reusable workflow.The documentation has been updated to reflect these changes and advise users to pin to immutable SHAs rather than using
@main.The use of
github.tokeninstead ofsecrets.GITEA_TOKENensures that the automatic job token is correctly used even when explicit secret forwarding is employed.These changes effectively address the security concern of over-sharing secrets while maintaining the functionality of the workflows. The implementation has been verified through the self-validation mechanism described in the PR, where the gadfly's own caller uses a local reusable ref, thus exercising the new explicit-secrets contract end-to-end.
🎯 Correctness — No material issues found
VERDICT: No material issues found
The changes correctly implement the least-privilege principle for secrets handling in GitHub Actions reusable workflows. The modifications are logically sound and address the stated security concern of over-sharing secrets. Key aspects verified:
required: false, allowing callers to explicitly forward only what's neededsecrets: inheritsecrets.GITEA_TOKENtogithub.tokencorrectly handles the automatic job tokenThe implementation correctly addresses the core correctness issue of secret scope without introducing new logical errors.
🧯 Error handling & edge cases — No material issues found
No material issues found.
The changes correctly implement least-privilege secret forwarding by:
workflow_call.secretsinreview-reusable.ymlsecrets: inheritwith explicit secret forwarding in caller workflowssecrets.GITEA_TOKENtogithub.tokensince the job token is automatically available via the GitHub context, not as a forwarded secretThe changes have been verified to work end-to-end by the PR's own review run as noted in the description. No error handling or edge case issues were introduced.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 20s
🪰 Gadfly review —
claude-code/opus:max(claude-code)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
I have enough to assess this through the Security lens. Let me note the verification I did:
review-reusable.yml(full) — confirmed theworkflow_call.secrets:block, allrequired: false, and theGITEA_TOKEN: ${{ github.token }}change at line 87.adversarial-review.yml(full) — confirmed the explicitsecrets:forward replacedsecrets: inherit, and the comment-trigger actor gate (if:) is unchanged.GITEA_TOKEN/github.tokenusages acrossscripts/,entrypoint.sh,.gitea/— confirmed how the token is consumed (run.sh,status-board.sh,entrypoint.sh).VERDICT: No material issues found
This PR is itself a security-hardening change (least-privilege secret scoping), and through the Security lens it holds up. Findings:
Secret exposure is reduced, not widened.
adversarial-review.yml:45-49forwards only 4 secrets vs. the previoussecrets: inherit(which handed the reusable every caller secret — registry/deploy/db creds). Theworkflow_call.secrets:declarations (review-reusable.yml:55-65) are allrequired: false, so an unforwarded secret resolves to empty and simply disables that provider — no auth-bypass or fail-open path. Verified by reading both files.secrets.GITEA_TOKEN→${{ github.token }}is sound and necessary, not a privilege change. Under explicit forwarding,secrets.GITEA_TOKENwould resolve empty inside the reusable (it's not a declared/forwardedworkflow_callsecret), which would break comment-posting auth.github.tokenis the same automatic job token, sourced from thegithubcontext rather than the secrets map, and is gated by the workflow'spermissions:block (contents: read,issues/pull-requests: write— line 69-72). Token scope is unchanged; the token is used only inAuthorization: tokenheaders and the clone URL (run.sh,entrypoint.sh:126), none of which this PR touches. Verified by grep + reading the consumers.Authz on the secret-bearing comment trigger is intact. The
issue_commentactor gate (adversarial-review.yml:36-41, restricting tosteve/fizi/dazed) is preserved and mirrored by the in-containerallowed_userscheck (line 57) — the defense-in-depth against an untrusted user triggering a secret-bearing run is unchanged by this diff.No new tainted-input path. This diff is YAML secret plumbing + docs; it introduces no new handling of untrusted PR/comment input (
github.event.comment.bodyetc. flows are unchanged), no injection sink, no SSRF, and leaks no credentials into logs or image layers.One advisory note within-lens: the docs correctly push consumers toward pinning
@<sha>instead of@main(review-reusable.yml:26-27,examples/reusable.yml), which closes the real supply-chain risk that a push to gadfly could change the code running against a consumer's forwarded secrets. That's the right hardening guidance and matches the explicit-secrets intent.Nothing in the Security lane is materially wrong here.
🎯 Correctness — No material issues found
I've verified the change against the actual code. Let me confirm my understanding of the consumption side is complete.
The central change is
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}→${{ github.token }}(review-reusable.yml:87), motivated by: under explicit forwarding,secrets.GITEA_TOKENis no longer auto-inherited, so it would resolve empty. I verifiedGITEA_TOKENis hard-required atentrypoint.sh:61(:?GITEA_TOKEN required) and used for all API/clone/comment calls — so if it resolved empty, the run would abort before posting.VERDICT: No material issues found
Through the correctness lens, this is a clean configuration change. My checks:
github.tokensubstitution (review-reusable.yml:87) — logically correct. Under explicitsecrets:forwarding,secrets.GITEA_TOKENis not a declaredworkflow_callsecret, so it would resolve to empty andentrypoint.sh:61would hard-abort. Sourcing from thegithubcontext token instead is the right fix. Verified the token's only consumers (entrypoint.sh,scripts/run.sh,scripts/status-board.sh) all read theGITEA_TOKENenv var, not the secret directly, so the swap is transparent to them. The declaredpermissions:block (issues/PR write) governs the token's scope unchanged.Forwarded-secret set matches the model fleet — verified. gadfly's own caller (adversarial-review.yml:46-49) forwards exactly
OLLAMA_CLOUD_API_KEY+CLAUDE_CODE_OAUTH_TOKEN(itswith.modelsuses*:cloudandclaude-code/*) plus the two findings vars. No provider key the configured models need is dropped. All 10 declaredworkflow_call.secrets(lines 56-65) have matching env mappings (lines 96-109).required: falseon every secret — correct. An unforwarded secret resolves to empty, which the entrypoint/image treat as "use built-in default," consistent with the pre-existing input semantics.One thing I could not verify (out of tool reach, not a defect I can confirm): that
github.tokenactually resolves non-empty on this Giteaact_runnerinside aworkflow_callreusable. This is the load-bearing runtime assumption of the whole PR. The change is designed to self-validate (the local-ref review run either posts a comment or hard-fails atentrypoint.sh:61), and the prior commit message indicates it was exercised — so I'm not flagging it as a bug, only noting it's the single behavior to confirm from the live run rather than from the diff.Stale-but-harmless: the env-block comment at review-reusable.yml:95 still says "via secrets: inherit" — cosmetic, no behavioral impact (belongs to a docs/maintainability lens, not correctness).
🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
I reviewed this change strictly through the error handling & edge cases lens. The diff is YAML-workflow + docs only; it introduces no new code paths, and I verified that the unhappy paths it touches are already handled gracefully in the consuming scripts.
What I checked (and why it's fine):
Unforwarded secrets resolving to empty. The PR's central claim is that an unset/unpassed secret "resolves to empty, harmless." Verified against the code: every provider key in
review-reusable.yml:96-108flows intorun.sh, which degrades rather than crashes.scripts/run.sh:131-138posts a⚠️ No Ollama Cloud key configured… skippednotice whenOLLAMA_CLOUD_API_KEY/GADFLY_API_KEYare empty; the claude-code/antigravity branches similarly emit⚠️notices and continue. So a consumer who forwards too few secrets gets a skipped-lens notice, not an unhandled error. Claim holds.adversarial-review.ymlforwards a sufficient set. The model list uses cloud models (needOLLAMA_CLOUD_API_KEY— forwarded) andclaude-code/*(needCLAUDE_CODE_OAUTH_TOKEN— forwarded). No model in the suite depends on an un-forwarded key, so even the graceful-skip path isn't triggered here. Andentrypoint.shruns each model under... || log "model failed (continuing)"(entrypoint.sh,review_one), so one bad lane can't abort the swarm.GITEA_TOKENsource switch (secrets.GITEA_TOKEN→github.token). This is the only functionally meaningful change. Bothentrypoint.sh:62andscripts/run.sh:43guard it with: "${GITEA_TOKEN:?GITEA_TOKEN required}", so an empty value fails fast and explicitly rather than silently posting with an emptyAuthorization: tokenheader. The failure-handling behavior here is unchanged from before — only the token's source changed — so no new edge case is introduced on this path.No swallowed errors, missing cleanup, nil/empty/boundary mishandling, or new panics are introduced by this diff within my lens.
Outside my lens (one line, flagging only): whether
github.tokenis actually non-empty inside a Giteaworkflow_calljob is the load-bearing assumption of this PR; if it ever resolves empty the:?guard turns the review into a non-zero exit — worth confirming under correctness/advisory-invariant review, which covers it.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 24s
steve referenced this pull request from steve/executus2026-06-28 01:23:39 +00:00