Skip to content

runner-policy: reusable contracts cannot be scoped by repo visibility, so a sensitive input enabled for a private consumer is enabled for public ones too #277

Closed
#355
Closed
runner-policy: reusable contracts cannot be scoped by repo visibility, so a sensitive input enabled for a private consumer is enabled for public ones too#277
#355
Assignees
kyle-sexton
Labels
area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: mediumReal value, no hard deadline; normal backlog flow.status: needs-decisionAwaiting a human or maintainer judgment call.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.

Description

@kyle-sexton
kyle-sexton
opened on Jul 26, 2026

The gap

approvedReusableWorkflowContracts entries in components/runner-policy/policy.json are keyed by path@SHA and are shared by every governed consumer repository. The schema has no way to scope a contract to a repository or to a visibility class.

allowedInputs is a permission, not a requirement — listing an input lets any governed caller pass it; omitting it forbids every caller from passing it. There is no middle setting.

So a contract that legitimately enables a sensitive input for a private consumer simultaneously enables it for every public consumer at that SHA.

Why it is not currently biting

The concrete case is claude-review.yml's standards-ref input, which mounts melodic-software/standards into the review agent via a GitHub App token. Today:

The gap is latent, not live. It becomes live the moment someone adds standards-ref to a contract "for the private consumers".

The invariant to record

No claude-review / claude-security-review contract may list standards-ref or the STANDARDS_REVIEW_APP_* secrets while any governed caller repository is public — unless the contract schema first grows repository- or visibility-scoped entries.

Two public governed callers exist today: ci-workflows and claude-code-plugins, plus standards itself (verified live 2026-07-26; see #276, which corrects the stale classification that said otherwise).

Options

  1. Record the invariant only — a note in the runner-policy README or distribution/THREAT-MODEL.md. Cheapest; relies on a human reading it at the moment they edit policy.json.
  2. Enforce it in the gate — teach runner-policy.mjs to fail when a contract lists a denylisted input and the evaluating repository is public. It already knows the repository's visibility (CI_REPOSITORY_VISIBILITY is passed to the job), so this is a local check, not new plumbing. Stronger, because it fires at the point of use rather than the point of authorship.
  3. Scope contracts by visibility in the schema — most general, most work, and unjustified until a second case exists.

Option 2 is the one that actually holds, since the failure mode is a future editor who does not read the note. Option 1 is worth doing regardless and immediately.

Trigger for revisiting

Any of: a private consumer genuinely needs the standards-ref mount; the schema gains repo- or visibility-scoped contracts; or REVIEW-CREDENTIAL.md is re-derived (per #276) into an actual provisioning plan.

Provenance

Surfaced during the security review of the @e2951077 contract approval (#275), as the one way that approval could go wrong later. The approval itself is sound — the entries omit the sensitive surface, which is exactly what makes the mount unpassable today.

Activity

  1. kyle-sexton
    self-assigned this
    on Jul 27, 2026

  2. kyle-sexton commented on Jul 27, 2026

    @kyle-sexton
    kyle-sexton
    on Jul 27, 2026
    MemberAuthor

    This was generated by AI during triage.

    Claiming this issue for a triage pass: verifying the body's load-bearing claims against the repository, then posting an executable brief.

  3. kyle-sexton commented on Jul 27, 2026

    @kyle-sexton
    kyle-sexton
    on Jul 27, 2026
    MemberAuthor

    This was generated by AI during triage.

    Triage pass: every load-bearing claim in the body was checked against the repository. Five hold; claim 4 — the one that makes Option 2 cheap — is refuted as written, and the correction is security-relevant. The body's public-caller enumeration also over-counts by one. Details below; a cold agent can execute from this comment alone.

    The invariant (corrected enumeration)

    No claude-review / claude-security-review contract in components/runner-policy/policy.json may list standards-ref or the STANDARDS_REVIEW_APP_* secrets while any repository that executes the runner-policy gate is public — unless the contract schema first grows repository- or visibility-scoped entries.

    Public gate-executing repositories today (verified live 2026-07-27 via gh api repos/melodic-software/<r> --jq .visibility):

    Repository Visibility Executes the gate?
    standards public yes — self-enforced, .github/workflows/ci.yml:198-200
    claude-code-plugins public yes — managed consumer, sync-manifest.yml:192
    ci-workflows public no — sync-manifest.yml:168-179 does not manage runner-policy
    dotfiles, github-iac, medley, provisioning private yes

    The body names ci-workflows as a public governed caller. It is public, and it is a sync target, but it does not manage the runner-policy component, so the gate never evaluates it. It is the producer of these reusable workflows and calls them locally. Two public repos execute the gate, not three.

    Verified-claims table

    # Claim Verdict Evidence
    1 Contracts keyed path@SHA, shared by every consumer, no repo/visibility scoping in the schema verified policy.schema.json:49-52 — propertyNames: reusableWorkflowReference, additionalProperties: reusableWorkflowContract. Both contract variants are additionalProperties: false (:182, :200), so no scoping field is even expressible. Keys are bare path@SHA (policy.json:447, :460). One policy.json is materialized to all five targets (sync-manifest.yml:138).
    2 allowedInputs is a permission, not a requirement verified runner-policy.mjs:1005 — Object.keys(inputs).filter((name) => !contract.allowedInputs.has(name)) rejects only inputs present but unlisted. Nothing requires a listed input. Sole exception: runnerInput is separately required at :1015, and validatePolicy forces allowedInputs to contain it at :169.
    3 @e2951077 contracts omit standards-ref and both STANDARDS_REVIEW_APP_* verified policy.json:447-459 — allowedInputs: ["runner","skip-actors"], allowedSecrets = CLAUDE_CODE_OAUTH_TOKEN only. policy.json:460-472 (claude-security-review) adds only paths.
    4 Gate already receives visibility via CI_REPOSITORY_VISIBILITY, so the check is local, not new plumbing REFUTED as written See the section below.
    5 REVIEW-CREDENTIAL.md says "classified, not provisioned" verified, with drift Substring is verbatim at distribution/REVIEW-CREDENTIAL.md:15, but the line continues "— re-derived 2026-07-26 as not currently needed", and :19-20 adds "under the current visibility state there is nothing to provision".
    6 All six named callers pass only runner and skip-actors (b) verified / (a) refuted All six confirmed (see per-caller lines below). But a seventh caller exists: ci-workflows/.github/workflows/claude-review-self.yml:31 via a local ./ ref with no with: block at all. claude-review.yml@ grep misses it.

    Claim 6 per-caller evidence

    All under D:/repos/github.com/melodic-software/, all passing exactly runner + skip-actors, secrets CLAUDE_CODE_OAUTH_TOKEN only, all clones verified present and clean on main:

    standards/.github/workflows/claude-review.yml:29, medley/…:60, provisioning/…:52, dotfiles/…:46, github-iac/…:53, claude-code-plugins/…:25. Five are pinned @90f1c54935203fa31b5b3d1f41531228be2c2b7f; claude-code-plugins alone is on @e2951077a7b43c09fc5a8dee4da52ba6f0fb39ed. Nothing could not be verified. (Suffixed worktree directories were excluded — a caller change could be pending in one.)

    The adjacent fact that matters most, which the body does not state: standards-ref (ci-workflows/.github/workflows/claude-review.yml:117, type: string, default: "") and both STANDARDS_REVIEW_APP_ID (:139) / STANDARDS_REVIEW_APP_PRIVATE_KEY (:146) already exist in the reusable workflow's workflow_call interface — byte-identically at both pinned SHAs. The mount steps are gated at :227, :241, :253 on inputs.standards-ref != ''. The surface is live in the workflow; the policy.json contract is the only thing standing between it and use. That is the strongest argument for Option 2 over Option 1 — Option 1 leaves a live workflow surface guarded by a note.

    Claim 4 — the critical finding

    Is the visibility value available at the input-validation site? No.

    • repositoryVisibility enters at runner-policy.mjs:3524 (values["repository-visibility"] ?? environment.CI_REPOSITORY_VISIBILITY) and is consumed at exactly one place: :3135-3142, guarded by if (repositoryVisibility) — an optional consistency check against config.visibility, nothing more.
    • The input-validation site is runner-policy.mjs:1005, inside reusableWorkflowStatus(job, policy, workflow) (:964, called at :2248 and :2757). Its policy argument is assembled at :3145-3153 and carries repositoryOwner and approvedSelectorReferences — not visibility. grep 'policy\.visibility' over the file returns zero hits. Neither config nor the env var is in scope there.
    • What is authoritative and in scope in the enclosing auditRepository body: config.visibility, required by repository-policy.schema.json:7 with enum ["public","private"] at :14, already read at :3212 and :3382.

    Cost verdict: Option 2's cost magnitude survives, its stated mechanism does not. Threading visibility: config.visibility onto the policy object at :3145 exactly mirrors how repositoryOwner is already threaded — roughly a one-line change. So the option is still cheap. But the difference is not cosmetic:

    A check written literally against CI_REPOSITORY_VISIBILITY fails open for any consumer that does not set it, because the variable is optional at :3135. The fix must read config.visibility.

    Empirically every consumer sets it today — claude-code-plugins/.github/workflows/ci.yml:916, dotfiles/…/ci.yml:343, github-iac/…/ci.yml:355, medley/…/ci-status.yml:492, provisioning/…/ci.yml:368, standards/…/ci.yml:200. But no workflow is in the sync manifest (sync-manifest.yml:135-141 lists only the two schemas, policy.json, runner-policy.mjs, package.json, package-lock.json). Each consumer owns its own enforcement step and its own env wiring, so nothing structurally guarantees the variable is passed — a new consumer or a local edit can silently drop it.

    The three options

    Option Cost Coverage
    1. Record the invariant only — note in components/runner-policy/README.md or a THREAT-MODEL.md trivial Zero enforcement. Fails against the actual threat: a future editor who does not read the note. Leaves a live workflow_call surface guarded by prose.
    2. Enforce in the gate — runner-policy.mjs fails when a contract lists a denylisted input/secret and the evaluating repository is public ~1 line of plumbing + one check + tests, per the trace above Fires at the point of use, in every governed repo, on every PR.
    3. Scope contracts by visibility in the schema Real schema work — both contract variants are additionalProperties: false (policy.schema.json:182, :200), so a new field plus validator plus migration Most general. Precedent partly exists: approvedSelectorReferencesByRepositoryOwner (runner-policy.mjs:3151) already owner-scopes selector references — but not contracts. Unjustified until a second case exists.

    Option 1 is settled — do it regardless, immediately, and it needs no decision.

    Option 2 is directionally settled but its mechanism is an open decision, because claim 4 was wrong about where the value comes from. config.visibility is consumer-declared in that repo's own .github/runner-policy.json; the env-var cross-check at :3139 is the only thing tying it to reality, and it is optional. A public repo that declares visibility: "private" and omits the variable bypasses a config.visibility-based gate entirely.

    Open decision — needs an operator answer. When a contract lists a denylisted input, must the gate require visibility evidence (fail closed if CI_REPOSITORY_VISIBILITY is absent), or is the consumer-declared config.visibility sufficient?

    • RECOMMENDED: fail closed, narrowly. Require the evidence variable only when a contract actually lists a denylisted entry. Absence stays harmless for every other repository and every other contract, so the blast radius of the stricter posture is confined to exactly the case the invariant is about — and the self-assertion bypass is closed.
    • Alternative: trust config.visibility unconditionally. Simpler, no new failure mode, but leaves a declare-private bypass on a security invariant.

    Exact files a fix would touch

    • components/runner-policy/runner-policy.mjs
      • auditRepository policy assembly, :3145-3153 — add visibility: config.visibility beside repositoryOwner.
      • reusableWorkflowStatus, :964 — new check adjacent to the allowedInputs filter at :1005. The check must fire on the contract's own listing, not on the caller's with: block, so it catches the surface at approval-effective time even while no caller passes it. Checking only job.with would leave the gap wide open.
      • Consider also validatePolicy (:155-235) if the denylist should reject at policy-load time; note that path has no visibility context, so it can only be an unconditional ban, not a visibility-scoped one.
      • Denylist constant needs a home: standards-ref, STANDARDS_REVIEW_APP_ID, STANDARDS_REVIEW_APP_PRIVATE_KEY.
    • components/runner-policy/runner-policy.test.mjs — new cases (below).
    • Option 1 target (do regardless): components/runner-policy/README.md and/or components/runner-policy/THREAT-MODEL.md; distribution/THREAT-MODEL.md if the invariant belongs at distribution scope.
    • components/runner-policy/policy.schema.json — only under Option 3.
    • Do not touch policy.json contract entries — the @e2951077 approval is correct as merged.

    Test strategy

    Real command, verified in package.json:27: npm run test:runner-policy → node --test components/runner-policy/runner-policy.test.mjs. Self-check on this repo: npm run lint:runner-policy (package.json:22) → node components/runner-policy/runner-policy.mjs --root ..

    Existing harness to reuse: fixture helper repository({ visibility, ... }) at runner-policy.test.mjs:105 (writes .github/runner-policy.json, defaults visibility = "private" at :107); wrapper audit(root, options) at :168. Nearest precedent test: "GitHub visibility evidence must agree with governed inventory" at :1037.

    Cases that must be covered:

    1. Public caller, contract lists a denylisted input → finding raised; assert the exact reason string, not just truthiness.
    2. Private caller, same contract → no finding. This is the false-positive guard; a bug here blocks CI in four private repos.
    3. Missing visibility variable. Expected assertion depends on the open decision above. Under the RECOMMENDED posture: audit() called without repositoryVisibility against a contract listing a denylisted entry must fail closed with a distinct reason naming the missing evidence — and the same call against a clean contract must stay green.
    4. Contract lists a denylisted input but no caller passes it → still a finding. This proves the check sits at contract level, which is the whole point of enforcing at the point of use.
    5. Regression: the current @e2951077 entries plus all seven real callers stay green.

    Failure-mode warning

    runner-policy.mjs is the CI gate every governed consumer runs on every pull request. An over-broad check fails the Enforce runner policy step in all five consumers plus standards, blocking every PR fleet-wide — and because policy.json is a single shared file, a check that trips on any one contract entry trips for every repository regardless of which one motivated it. Recovery is not instant: the corrected runner-policy.mjs has to travel back out through a standards-sync PR per consumer.

    Staging a fix, in order:

    1. Land the check upstream with npm run test:runner-policy green.
    2. Verify against standards itself: npm run lint:runner-policy with CI_REPOSITORY_VISIBILITY=public (public path).
    3. Verify the private path before any sync PR — run the upstream runner-policy.mjs --root <path> against at least one private consumer clone (dotfiles, github-iac, medley, or provisioning) and confirm no new finding.
    4. Only then let the sync PRs reach consumers, and watch the first consumer PR to green before merging the rest.

    Blast radius

    runner-policy is a sync-managed distributed component (distribution/sync-manifest.yml:132-141). runner-policy.mjs (:141), policy.json (:138), and both schemas (:139-140) are managed files, materialized to .github/standards/runner-policy/ in five targets — claude-code-plugins (:192), dotfiles (:214), github-iac (:231), medley (:238), provisioning (:257). Per the repo's own convention these materializations are upstream-owned — the change belongs here, never in a consumer's copy.

    One asymmetry worth flagging: runner-policy.test.mjs is not in the manifest. Consumers receive gate code with no test materialization, so upstream tests are the only proof the gate is correct before it ships.

    Related

    #276 is a merged PR, not an open issue — docs(distribution): correct the review-credential visibility facts. It establishes that standards and claude-code-plugins are public (correcting a "confirmed firsthand" private classification), voids the credential's stated premise, and records that the proposed org-secret visibility lists would have made both STANDARDS_REVIEW_APP_* secrets resolvable in public-repo workflow runs. It deliberately leaves the superseded scope lists unedited so a re-derivation sees the original proposal. It is resolved; nothing in it is pending.

    Out of scope

    • Provisioning the review credential — re-derived as not currently needed (REVIEW-CREDENTIAL.md:15).
    • Option 3's schema change — deferred until a second case exists.
    • Any change to a caller workflow, or to ci-workflows' standards-ref implementation.
    • Any edit to policy.json contract entries.

    Acceptance criteria

    1. The invariant is recorded in a durable doc with the corrected enumeration (two public gate-executing repos; ci-workflows is public but not gate-governed).
    2. The gate rejects standards-ref / STANDARDS_REVIEW_APP_* in a contract when the evaluating repository is public, reading config.visibility — not CI_REPOSITORY_VISIBILITY directly.
    3. The check fires on the contract's listing, not on a caller's with: block.
    4. The missing-visibility posture matches whatever the operator decides on the open decision above, with a test asserting it.
    5. All five test cases pass; npm run test:runner-policy green; npm run lint:runner-policy green on standards.
    6. The four-step staging path is followed before any sync PR reaches a consumer.

    Triage disposition

    Labels applied: priority: medium, area: security, work-class: structural, needs-human, status: needs-decision.

    Priority is medium, not high, and the body's own "latent, not live" framing is the reason — with the one correction that the latency is thinner than described. No caller passes the surface, no contract lists it, and the credential is re-derived as unneeded; but the input and both secrets do exist in the reusable workflow at both pinned SHAs, so the only thing holding the gap shut is a policy.json entry that a single well-intentioned edit could widen. Escalate to priority: high on any of the body's triggers: a private consumer genuinely needing the mount, the schema gaining scoped contracts, or REVIEW-CREDENTIAL.md being re-derived into an actual provisioning plan.

    needs-human and status: needs-decision because the enforcement mechanism is unresolved on a security invariant, and work-class: structural because the change lands in a distributed gate that five consumer repositories execute on every PR — a false positive blocks CI fleet-wide, a false negative leaves a credential-exposure path open.

  4. kyle-sexton
    added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.
    work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    status: needs-decisionAwaiting a human or maintainer judgment call.
    on Jul 27, 2026
  5. kyle-sexton
    removed their assignment
    on Jul 27, 2026

  6. kyle-sexton commented on Aug 11, 2026

    @kyle-sexton
    kyle-sexton
    on Aug 11, 2026
    MemberAuthor

    Frontier consensus (Fable + Sol) — ACCEPT

    Decision: Fail closed when CI_REPOSITORY_VISIBILITY is absent for denylisted sensitive inputs; ship Option 1 docs in the same PR wave.

    Implementing in-session.

    🤖

  7. kyle-sexton
    self-assigned this
    on Aug 11, 2026

  8. kyle-sexton commented on Aug 11, 2026

    @kyle-sexton
    kyle-sexton
    on Aug 11, 2026
    MemberAuthor

    Lease claim for in-session drain. Implementing now. 🤖

  9. cursor
    added 2 commits that reference this issue on Aug 11, 2026

    runner-policy: enforce visibility-scoped Claude lane contract invariant

    944d93c

    runner-policy: enforce visibility-scoped Claude lane contract invariant

    f9e7548
  10. kyle-sexton
    closed this as completedin #355on Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: mediumReal value, no hard deadline; normal backlog flow.status: needs-decisionAwaiting a human or maintainer judgment call.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions