fix(cla): declare workflow_call.secrets so callers can stop using secrets: inherit - #22
Merged
Merged
Conversation
…crets: inherit`
Every migrated caller passes `secrets: inherit` because this workflow reads
`secrets.GH_PERSONAL_ACCESS_TOKEN` without declaring it. `inherit` forwards
EVERY secret the calling repo holds into a job started by `pull_request_target`
or `issue_comment` -- events an outsider can trigger with no write access.
Declaring the one secret this workflow actually needs lets a caller narrow to:
secrets:
GH_PERSONAL_ACCESS_TOKEN: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN }}
BACKWARD COMPATIBLE: `secrets: inherit` continues to work, so no caller
breaks and repos migrate when they choose.
`secrets.GITHUB_TOKEN` needs no declaration -- it is provided to called
workflows automatically.
This does NOT touch `runs-on`. Whether this workflow should run on
spartan-cpu is a separate decision and belongs to whoever owns the
no-hosted-runners directive.
Requested by sac on the PR. A maintainer migrating to this template sees `spartan-cpu` and no reason to question it; sac's own cla.yml cannot be 'fixed' by a well-meaning agent without reading why first. This gives the template the same protection. Records the four facts that only bite in combination, the measured severity bound (no injection path found; hardening, not compromise), what remains unverified, and that sac's own repo refused this shape with a written PS-224 exemption calling the move a REGRESSION. No behaviour change - comments only. runs-on unchanged.
ywatanabe1989
added a commit
to scitex-ai/scitex-dev
that referenced
this pull request
Jul 29, 2026
PS-224's new caller-side check found a TRUE finding in this repo and this commit fixes the workflow rather than the rule. The org reusable now declares `workflow_call.secrets` (scitex-ai/.github#22, commit ecb0345b), so a caller can finally name the single secret the callee reads. Before that, `secrets: inherit` was the only option available. Two coupled changes, because they cannot be separated: passing a named secret to a callee that does not declare it is an error, so the pinned callee ref moves 399f19be -> ecb0345b in the same commit. The file's own policy is to pin the SHA and take reusable updates deliberately; this is one such deliberate update, and the reason is recorded in the file. Measured on this repo, `secrets: inherit` was forwarding TWO secrets: GH_PERSONAL_ACCESS_TOKEN the callee actually reads it CLAUDE_CODE_CREDENTIALS_JSON the fleet Claude credential, no CLA use The second had no business being in the environment of a job an unauthenticated stranger can start via `pull_request_target` or `issue_comment`. Org-level secret list is empty, so that count is per-repo -- two is this repo's number, not a fleet number. Verified, including the control: PS-224 findings on this repo 1 -> 0 control, same file reverted to `inherit` STILL FIRES narrowed shape does not fire triggers still detected issue_comment, pull_request_target The control is the point. A rule going quiet after an edit is indistinguishable from a blinded detector unless you show the unnarrowed shape still trips it. What this does NOT fix, stated in the file so a later reader does not mistake a quiet rule for a safe job: the callee still runs on the persistent shared runner with the PAT in the job environment. Narrowing shrinks blast radius; it does not remove credential adjacency, and neither file shows that adjacency on its own. PS-224 reads one file at a time, so the residual is tracked as a rule gap.
ywatanabe1989
added a commit
to scitex-ai/scitex-dev
that referenced
this pull request
Jul 29, 2026
… secret jobs (#475) * fix(audit): PS-224 told you to move attacker-triggerable secret jobs onto the credential box Sixteen rule modules read `.github/workflows/`. Zero modelled the TRIGGER: `pull_request_target`, `workflow_run` and `issue_comment` appeared nowhere in the audit corpus. The corpus read workflows along two axes — how a secret is NAMED (PS-168, which blesses `GH_PERSONAL_ACCESS_TOKEN` as canonical) and where a job RUNS (PS-224). Trigger is the axis that decides whether either matters, and it was unrepresented. Two rules, each correct alone, combined into: "that correctly-named fleet PAT — please run it on the shared box." Measured: 619 workflow files across 112 repos, 434 carrying a hosted label, 70 of those also attacker-triggerable AND secret-bearing. Adds `_workflow_exposure.py`. Handles the YAML gotcha that hides triggers from every naive reader: bare `on:` parses as the BOOLEAN True, so `doc.get("on")` silently returns nothing for most real workflows — knowledge `_check_hosted_runners` already had and had never shared. Secret detection is a text scan of the serialized job rather than a walk of known secret-bearing keys, because a secret reaches a step via `env:`, `with:`, an inline `run:` or a `uses:` input, and enumerating those sites is the kind of incomplete list this module exists to stop relying on. PS-224 now branches. Unexposed jobs keep the original remedy verbatim. Exposed jobs are told DO NOT RE-POINT, with the triggering event and the specific secret named. The exposed branch withholds a remedy rather than choosing one. The safe options are to keep the job hosted — an exception to the no-hosted-runners directive, which states no exceptions — or to stop needing the credential, which changes what the workflow can do. Neither is the rule's to pick, so it reports the fact, withholds the remedy, and escalates. A rule that cannot name a safe fix should state that plainly rather than name an unsafe one. Severity is rule-keyed, so PS-224 cannot emit this class as a warning; the gate stays red for these until the directive owner decides. The positive control is the test that matters: not that the new text is present, but that "re-point the job" is ABSENT. A fix that appended a warning while leaving the original remedy would pass a presence-only test and still ship the advice that caused the finding. * docs(audit): state the cross-file exposure bound this reader cannot see A reusable workflow declares `on: workflow_call` while the `pull_request_target` reaching it lives in the CALLER. Read alone the called file looks safe; the calling file has no `runs-on` to flag. The exposure is real and neither file shows it. Measured on this repo's own cla.yml, whose single `uses:` job reports exposed=False. Closing it needs a cross-file call graph. Until then the bound is stated, with a test pinning current behaviour so a future fix has an anchor: absence of exposure means 'not visible in this file', never 'not exposed'. Found by scitex-app while checking their own cla.yml against the fix. * fix(audit): flag the delegating caller — the blind spot is the common case, not an edge The previous commit documented the cross-file gap as a bound. scitex-app checked whether it was real and found it already live on the ORG TEMPLATE every migrated repo calls. Verified independently before acting. CALLER (scitex-app@main .github/workflows/cla.yml): on: issue_comment, pull_request_target <- attacker-triggerable jobs.call: uses: scitex-ai/.github/.github/workflows/cla.yml@main secrets: inherit <- fleet PAT propagates no `runs-on` anywhere -> PS-224 never looked at this job CALLEE (scitex-ai/.github, fetched): on: workflow_call <- looks safe read alone line 49/68: runs-on: [self-hosted, Linux, X64, spartan-cpu] line 76: PERSONAL_ACCESS_TOKEN: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN }} So the migration has ALREADY produced the end-state this rule must never recommend, reached by another route, invisible from either file alone. A per-file reader returns "not exposed" for every repo in the fleet on the one workflow where the answer is yes — wrong in the single direction that matters. The bound was describing the common case. `secrets: inherit` on an attacker-triggerable caller is decidable from the CALLER ALONE: it means "delegate execution with every secret this repo holds" without naming one. So this needs no call graph and ignores `runs-on` entirely — the caller has none, which is exactly how the job stayed invisible. PS-224 previously skipped every job without `runs-on`. It now flags this shape, and the docstring's "the one case that is NOT a violation" is corrected to match. Severity bounded, not assumed: scitex-app checked for a code-injection path and found none (no checkout of PR head in the callee; the sole third-party action is SHA-pinned). What remains is attacker-triggerable execution on a persistent shared runner with a fleet PAT in the environment — a hardening concern. The finding text says to verify where the callee runs rather than asserting a compromise. Fit under the 512-line cap by collapsing two imports into a module import, not by using the refactor escape hatch. 19 tests. The negative controls matter as much as the positive: a delegating caller WITHOUT an attacker-triggerable trigger is not flagged (`secrets: inherit` alone is not the defect), and a non-delegating job is not flagged as delegating. Reported by scitex-app; caller-side signal was their suggestion. * fix(ci): narrow CLA caller to one secret, unblocked by org template PS-224's new caller-side check found a TRUE finding in this repo and this commit fixes the workflow rather than the rule. The org reusable now declares `workflow_call.secrets` (scitex-ai/.github#22, commit ecb0345b), so a caller can finally name the single secret the callee reads. Before that, `secrets: inherit` was the only option available. Two coupled changes, because they cannot be separated: passing a named secret to a callee that does not declare it is an error, so the pinned callee ref moves 399f19be -> ecb0345b in the same commit. The file's own policy is to pin the SHA and take reusable updates deliberately; this is one such deliberate update, and the reason is recorded in the file. Measured on this repo, `secrets: inherit` was forwarding TWO secrets: GH_PERSONAL_ACCESS_TOKEN the callee actually reads it CLAUDE_CODE_CREDENTIALS_JSON the fleet Claude credential, no CLA use The second had no business being in the environment of a job an unauthenticated stranger can start via `pull_request_target` or `issue_comment`. Org-level secret list is empty, so that count is per-repo -- two is this repo's number, not a fleet number. Verified, including the control: PS-224 findings on this repo 1 -> 0 control, same file reverted to `inherit` STILL FIRES narrowed shape does not fire triggers still detected issue_comment, pull_request_target The control is the point. A rule going quiet after an edit is indistinguishable from a blinded detector unless you show the unnarrowed shape still trips it. What this does NOT fix, stated in the file so a later reader does not mistake a quiet rule for a safe job: the callee still runs on the persistent shared runner with the PAT in the job environment. Narrowing shrinks blast radius; it does not remove credential adjacency, and neither file shows that adjacency on its own. PS-224 reads one file at a time, so the residual is tracked as a rule gap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Declares
workflow_call.secretsfor the one secret this workflow actually reads, so callers can stop passingsecrets: inherit.Backward compatible.
secrets: inheritkeeps working, so no caller breaks and repos migrate when they choose.secrets.GITHUB_TOKENneeds no declaration — it is provided to called workflows automatically.Why
Every migrated repo's caller looks like this, because this workflow reads
secrets.GH_PERSONAL_ACCESS_TOKENwithout declaring it:inheritis the only option available to a caller today. It hands the full secret set of the calling repo to a job an outsider can start by opening a PR or typing a comment. Narrowing to the single secret the workflow needs cuts the blast radius without changing what the workflow can do.What this does NOT change
runs-onis untouched — both jobs still target[self-hosted, Linux, X64, spartan-cpu]. Whether an attacker-triggerable, credential-bearing job should run on the shared box is a separate decision, it belongs to whoever owns the no-hosted-runners directive, and it is deliberately not bundled here. This PR is the half that improves things under either outcome.Severity, bounded
Measured by scitex-app, not by me, and labelled as theirs: no
actions/checkoutof the PR head anywhere in this workflow, and the sole third-party action is SHA-pinned. No code-injection path was found. What remains is attacker-triggerable execution on a persistent shared runner with a fleet PAT in the job environment — a hardening concern, not an active compromise.Explicitly unverified by anyone: the pinned action's contents, and whether a fork PR actually reaches these jobs.
Verification
workflow_call.secretsresolves to{'GH_PERSONAL_ACCESS_TOKEN': {'required': True}}inputsunchanged:default_branch,owner_allowlist,owner_loginruns-onconfirmed unchangedContext
scitex-dev's PS-224 rule was telling repos to move workflows like this one onto a registered (Spartan) destination — advice that is backwards for this shape. That scope bug is fixed separately in scitex-dev#475. This PR is the org-side half: it makes the safer caller configuration possible, which it currently is not.
Not merging this myself despite having write access — the template is sac's and the runner question is unresolved.