Skip to content

fork: outside PRs run nothing, one checkout without actions/checkout, contract tests - #12

Merged
scuttletoby merged 2 commits into
mainfrom
fork/followup-guards
Nov 12, 2026
Merged

scuttletoby merged 2 commits into
mainfrom
fork/followup-guards

Conversation

@scuttletoby

@scuttletoby scuttletoby commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

Follow-up to #3, #4 and #8: small leftovers from their reviews, plus one owner request.

What changes

  1. Outside pull requests run none of their code here, and fail. Every job in fork-tests.yml and fork-checks.yml
    now starts with
    (!github.event.pull_request || github.event.pull_request.head.repo.full_name == github.repository). A deleted head
    repository reads as null, so the job doesn't run. The exception (fix round r2) is outside_pr, which runs only for
    an outside PR, checks nothing out, has permissions: {} and exits 1. In fork-tests.yml, fork_ci_result runs too,
    needs it and fails, so a required check never goes green on an untested outside PR. In fork-checks.yml
    (pull_request_target, always main's copy), the guard is enforced by main. In fork-tests.yml (pull_request, the
    PR's own copy), an outside PR can delete the guard, and its code runs once a maintainer approves the run. There the
    approval setting is the real protection, so it must stay on, and outside runs of fork-tests must never be approved.
  2. fork: green main again (schema-check comments, checkout without submodule cleanup) #8 minor 1. fork-schema-check.test.cjs now checks whole run: | blocks for ${{ }}, not only the run: line.
  3. fork: green main again (schema-check comments, checkout without submodule cleanup) #8 minor 2. The token ban now includes github.token.
  4. fork: green main again (schema-check comments, checkout without submodule cleanup) #8 minor 3. The four actions/checkout steps in fork-release.yml, and the four with the same bug in
    fork-follow.yml, are replaced by one shared plain-git step. It fetches one ref anonymously with
    --no-recurse-submodules, with full history (blob:none) or only the tip, plus an optional no-cone sparse set,
    then detaches. It fails on an empty REF, an unknown ref, a bad HISTORY or a workspace that already has a checkout.
    .git/config holds no credential.
  5. fork: follow each stable upstream release with the patch series #4 minor a. The old check in fork-follow.test.sh looked only for continue-on-error. It is replaced by one that
    catches every way to swallow a refusal: || true, || :, || exit 0, ; true, a trailing true, &, !,
    if …; then, set +e, set +o errexit, a trap, continue-on-error on the step or job, an if: on a refusal step,
    another shell or defaults, and BASH_ENV at workflow, job or step level.
  6. fork: follow each stable upstream release with the patch series #4 minor b. Every fork script that calls gh must source fork-gh-guard.sh at the top level before its first
    call. A non-shell script that calls gh fails the check. Every fork-*.test.sh must call fork_test_gh before
    anything except set-up lines, and a non-shell test may not spawn gh or a guarded script.
    • The three shell tests now set up the stub first. fork-secret-scan.test.sh gets a stub that refuses every call.
    • fork-follow.test.sh now builds its refusal list from the scripts that source the guard, so fork-release.sh
      is included.

The new .github/scripts/fork-contracts.test.py holds items 1, 4, 5 and 6. Each rule runs on the real files and on
broken copies that must fail.

Red before, green after

The new tests run against main's files in a scratch copy, or against main plus one mutation:

Item Before (main) After (this branch)
1 pr-guard: every job a pull request can start … FAIL (every job of both files) ok
2 main's shape test passes with ${{ github.event.pull_request.title }} inside a run: block the workflow runs … never runs pull request code FAIL on it; the mutant test is ok
3 main's tests: 49/49 pass with GH_TOKEN: ${{ github.token }} in an env FAIL on it
4 main's fork-release.yml: checkout: no actions/checkout drops credentials … and … preflight, build_bundle, assemble and publish use it FAIL; main's fork-follow.yml: the same for guard, plan, advance and report ok; the e2e runs each job's REF/HISTORY/SPARSE
5 main's fork-follow.test.sh "all passed" with || true on the guard step refusals: no refusal … can be swallowed FAIL
6 main's three shell tests: gh-guard: every fork shell test sets up the gh stub … FAIL; fork-workflows.sh without its guard line: … sources fork-gh-guard.sh first FAIL ok

All fork tests pass locally with GH_TOKEN and GITHUB_TOKEN unset and GH_HOST=bogus.invalid. actionlint 1.7.12, the
CI's version, run with shellcheck, reports nothing. shellcheck adds no warning compared with main. oxfmt (vite-plus
1.4.0) is clean.

Checkout against the real fork: I ran the shared step anonymously (empty git config, no credential helper, no
token) against https://github.com/scuttletoby/t3code, main 8b0ad2c:

Step Time Result
build_bundle (tip, /* !/.repos/) 18 s no .repos on disk
publish (tip, /.github/scripts/) — only .github/
assemble (full) 36 s 10540 commits

All three ran from that main, which has 2 gitlinks and no .gitmodules. git submodule foreach (what actions/checkout's
clean-up runs) fails on that checkout with No url found for submodule path '.repos/alchemy-effect/submodules/distilled'.
No release was made and nothing was published.

What happens when this runs again, stops half-way, or meets existing, old or empty state?

Case What happens Test
Re-run of a job (Re-run failed jobs) The checkout runs in a fresh workspace and fetches the same REF again checkout e2e fork-*.yml/<job> (each job's own values)
A step re-run in a workspace that already holds a checkout (half-applied) Fails; it never mixes two checkouts checkout e2e: a workspace that already holds a checkout fails
Empty state: empty REF Fails the step (actions/checkout would have fallen back to the event's SHA) checkout e2e: an empty REF fails the step
Old or gone ref (a deleted branch, a SHA not on the server) Fails the step checkout e2e: an unknown SHA / an unknown branch fails the step
Old state: upstream's tree with gitlinks and no .gitmodules Checks out; git submodule foreach fails on the same fixture checkout e2e: the fixture breaks git submodule foreach …
Empty history: tip vs full Tip is shallow; full reaches the first commit (blobs on demand) checkout e2e: full history reaches the first commit
A bad HISTORY value Fails the step checkout e2e: an unknown HISTORY fails the step
A PR whose head repository was deleted head.repo is null, so it counts as an outside PR. The guarded jobs don't run, and outside_pr and fork_ci_result fail (r2) pr-guard: the guard compares the head repository …, outside_pr's if is the guard's exact negation
A rule check that stops seeing its problem Mutants keep it honest: an empty or dropped check goes red every … fails line in fork-contracts.test.py and the schema-check mutant test

Item 1: what the sweep can and can't disable

Workflows that are registered and could be started from outside are all disabled_manually:

  • pull_request_target: pr-limits, pr-size, pr-vouch, relay-preview-cleanup, desktop-macos-preview-publish.
  • pull_request: ci, web-preview, relay-preview, the mobile ones, agent-context-budget.

Upstream workflows on main that are not registered can't be disabled: GET actions/workflows/<file> returns 404. #4's
sweep (fork-workflows.sh) runs only from fork-follow, daily or on dispatch, so it can disable a file only on the run
after GitHub registers it. That first run is not stopped. The ones an outside event could start:

File Trigger Why nothing runs from an outside PR now
mobile-eas-preview-fork.yml pull_request_target: labeled Needs a maintainer to add the label, a blacksmith runner the fork doesn't have, and EXPO secrets the fork doesn't have
desktop-macos-preview.yml pull_request: labeled Needs the label preview:mac; pull_request also waits for approval; needs a blacksmith runner
mobile-fingerprint-label.yml workflow_run of "Mobile Fingerprint Check" That workflow is disabled
thread-transfer-report.yml workflow_run of "CI" ci.yml is disabled; no fork workflow is named "CI"

issue-labels.yml runs on push and dispatch only, and Issues are off on this fork. If the owner wants this closed
rather than relying on labels and runners, the fork patch could delete mobile-eas-preview-fork.yml and
desktop-macos-preview.yml. This PR doesn't do that.

Fix round r2 (after the two security lenses)

  1. Outside PRs fail, never skip to green. A new job, outside_pr, in fork-tests.yml and fork-checks.yml. Its
    if is the guard's exact negation. It checks nothing out, has permissions: {}, gets no secrets, has one step and
    exits 1 with outside PR: not tested here; a maintainer must review. fork_ci_result now runs with always() and
    needs outside_pr. Its first lines refuse when OUTSIDE_PR is true, and its jq lets outside_pr pass only as
    skipped. The contract test holds that shape exactly, with 12 mutants. It also runs fork_ci_result's script on five
    result sets: outside PR with the rest skipped, outside PR with the rest passed, outside_pr failed on an own PR,
    own PR all passed, and a push.
  2. Refusal steps are whitelisted. A refusal step (the guard, the Environment re-check, the sweep before the move)
    may have only id, name, env and run. Its run may hold only mkdir -p "$RUNNER_TEMP/reports" and one script
    call. Before it in its job may stand only the shared checkout, another refusal, or one echo "x=…" >> "$GITHUB_OUTPUT"
    line. No earlier step may use an action, write GITHUB_ENV, GITHUB_PATH or BASH_ENV, or define a function. No
    BASH_FUNC_* env at any level. The old blacklist stays only for the other script steps. In gh-guard, the guard line
    must stand at the top level, after nothing but #!, set and comments.
  3. ${{ }} in run scripts. runScripts now takes every line indented past the run: key, whatever the value
    looks like. That covers run: alone with the script on the next line, and a plain scalar continued over later
    lines.
  4. gh calls split by \ are joined before matching.
  5. Checkout fetches refs/heads/<REF> unless REF is a 40-hex SHA. A tag named like the branch is never taken, and a
    refs/tags/… REF fails. e2e: the origin has a tag main on its older commit.
  6. Comments. fork-tests.yml now says the guard stops only honest PRs and the approval setting is the protection.
    The shared step no longer says every fork job uses it: fork-tests.yml keeps its 12 actions/checkout steps.
    Moving them isn't small. Most have an empty ref that falls back to the event's commit, and they use
    fetch-depth: 0 and 2 and their own sparse sets. They follow upstream's ci.yml step for step, which the drift
    check compares.

r2: what happens when this runs again, stops half-way, or meets existing, old or empty state?

Case What happens Test
An outside PR's run approved, or re-run outside_pr fails again and fork_ci_result fails again, on every attempt fork_ci_result's script: an outside pull request …
A run from before r2, re-run It uses its own older workflow file, so an outside PR's old run still skips to green. Only a writer can re-run it none (GitHub's re-run rule)
"Re-run failed jobs" on an own-branch PR outside_pr stays skipped. fork_ci_result re-runs and sees the same results … own-branch pull request with every job passed passes
Empty state: a push or workflow_call (no pull_request) OUTSIDE_PR is empty and outside_pr is skipped. fork_ci_result works as before … a push (OUTSIDE_PR empty) … passes
A cancelled run fork_ci_result (always()) sees cancelled results and fails, as before jq: only success, or skipped with a reason, passes
Old state: a tag named like the series or input branch The checkout takes the branch. A refs/tags/… REF fails the step checkout e2e: a branch name takes the branch …, a tag's full ref fails the step
A branch deleted between plan and checkout refs/heads/<name> is missing, so the step fails (as before) checkout e2e: an unknown branch fails the step
A step re-run in a workspace that already holds a checkout Fails at git remote add, as before checkout e2e: a workspace that already holds a checkout fails
A refusal step that changes later (a new line, a new step before it) The contract test fails until the whitelist is changed on purpose the refusal mutants

Not done (owner's call)

  • Deleting thread-transfer-report.yml and mobile-fingerprint-label.yml in the fork patch. Both start on
    workflow_run of a workflow name ("CI", "Mobile Fingerprint Check"). An outside PR can add a workflow with that
    name, and once its run is approved, its completion starts these from main with pull-requests: write. They read the
    artifact as data, so the impact is limited to a PR comment's text.

Notes for the owner

  • fork-tests.yml is disabled_manually since 2026-11-12T14:12:38Z, about a minute after main's push run
    43234574679. No Actions run explains it. This PR doesn't change it, because repo settings are off limits, but its
    Fork tests won't run until the workflow is enabled again.
  • Fixed in r2: an outside PR used to skip fork_ci_result, and a skipped job counts as passing for a required check.
    Now outside_pr and fork_ci_result fail on it. For fork-checks.yml, a ruleset that requires "No database schema
    changes" must also require "Outside pull request" (it is skipped, so green, on our own branches).
  • fork-checks-proof.yml (the proof workflow from fork: green main again (schema-check comments, checkout without submodule cleanup) #8) is still registered and active.

🤖 Generated with Claude Code

… contract tests

- fork-tests.yml, fork-checks.yml: every job runs only for a pull request whose head is in this
  repository (no pull_request_target run for outside forks).
- fork-release.yml, fork-follow.yml: the eight actions/checkout steps (persist-credentials: false
  fails on upstream's gitlinks without .gitmodules) are one shared plain-git step: anonymous
  fetch of one ref, full (blob:none) or tip history, optional sparse set, fails closed.
- fork-contracts.test.py: the pull request guard on every PR-started job and an allowlist of
  fork triggers; the shared checkout run end to end on a repository with a gitlink and no
  .gitmodules; no refusal in fork-follow.yml swallowed (|| true, || :, ; true, set +e, trap,
  continue-on-error, if:, another shell, BASH_ENV); every script calling gh sources
  fork-gh-guard.sh, every shell test sets up the gh stub first. Each rule with broken copies.
- fork-schema-check.test.cjs: whole run: | blocks are scanned for ${{ }}, github.token banned.
- The three shell tests set up the gh stub before anything else; fork-secret-scan.test.sh gets
  one that refuses every call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@scuttletoby

Copy link
Copy Markdown
Owner Author

lens: security+correctness S-1 · reviewer: opus high (t3lens-12-s1) · read at: 18dadd6
covered: the whole diff against main 8b0ad2c (4 fork workflows, fork-contracts.test.py, fork-schema-check.test.cjs, fork-release-workflow.test.py, the 3 shell tests, fork-test-gh.sh, fork-gh-guard.sh, fork-ci-tests.txt), plus what it calls: fork-release.sh resolve, fork-follow-push.sh, fork-follow.test.sh's workflow checks, the triggers of every fork workflow, and the repo's Actions settings (approval_policy all_external_contributors; rulesets [] and main not protected).
checks:

  • Tests in a scratch clone with GH_TOKEN/GITHUB_TOKEN unset and GH_HOST=bogus.invalid. At the head, all 7 listed fork tests pass (contracts, release-workflow, schema-check 51/51, ci-tests 8/8, follow, release, secret-scan).
  • Red before: the head's fork-contracts.test.py on main's files fails 10 checks (pr-guard, checkout, e2e, gh-guard), as the table says. Main's fork-follow.test.sh still says "all passed" with || true on the guard step; the head's contracts test fails on it.
  • Breaking each fix by hand at the head turns it red: the guard removed from fork_ci_result; main's fork-release.yml put back (contracts and release-workflow tests both fail); the guard line removed from fork-workflows.sh; || true on the guard; if: always() on plan; advance's needs.plan.result == 'success' dropped (fork-follow.test.sh catches both job-level gates).
  • Guard logic: X && (always()) is safe because && short-circuits. A deleted head repo gives null, so the job is skipped. workflow_call from fork-release/fork-follow inherits a schedule or dispatch event with no pull_request, so the guard passes. Only fork-checks.yml (pull_request_target) gives the guard real force; see finding 1.
  • CI: fork-tests.yml is disabled, so none of the new tests ran. The only check on the PR is "No database schema changes", red by design ("changes the schema check itself", owner merges past). It ran from main's copy, without the guard. Had fork-tests run, its workflows job runs every listed test (fork-ci-tests.txt now includes fork-contracts.test.py), and that would have caught everything in "Red before".
  • State question. Re-run of a job: hosted runner, fresh workspace, same REF fetched again. Workspace that already holds a .git: fails at git remote add (tested by the e2e). Workspace with loose non-git files: they are kept, with no git clean the way actions/checkout did it; this can't happen on hosted runners. Re-run of an old run from before the guard: it uses that run's own workflow file. Old fork-checks runs only read text, so they are harmless. An old fork-tests run of an outside PR runs without the guard, but only after a maintainer re-runs or approves it. Empty REF, unknown ref, bad HISTORY: fail (e2e). Re-run of the release after a half-done publish: this PR doesn't change it; publish re-fetches github.workflow_sha.
  • Skipped equals green: an outside PR's fork_ci_result is skipped, which counts as passing for a required check. Nothing is required today (no ruleset, no protection), so today this is only a note. It matters when a ruleset is added (the PR body says so).
  • Not changed here: fork-tests.yml keeps 12 actions/checkout steps with the default persist-credentials. Its contents:read GITHUB_TOKEN therefore sits in .git/config, where the tested code can read it (push and same-repo PR runs, read-only, public repo). The new step's comment "Every fork job checks out with this same step" isn't true for fork-tests.yml.
    finding: minor .github/workflows/fork-tests.yml:56 Under the pull_request trigger, GitHub runs the copy of fork-tests.yml from the PR's merge commit. An outside PR that deletes the if: guards in its own copy therefore runs all its jobs after a maintainer clicks Approve, on GitHub-hosted runners with the read-only token and no secrets. The comment "runs nothing here, not even after approval" is wrong for this file, and fork-contracts.test.py can't see the gap because it reads our copy. The real boundary is the approval setting, so it must stay on and outside fork-tests runs must never be approved. Fix: reword the comment and the PR body. (high)
    finding: minor .github/scripts/fork-schema-check.test.cjs:459 runScripts treats a run: key with no value, or a one-line run: continued on the next line, as a one-line script, and never reads the continuation. Tested at the head: run: with echo ${{ github.event.pull_request.title }} && node ... on the next line, and a one-line run with ${{ github.head_ref }} continued onto the next line. In both, "never runs pull request code" stays green, although YAML reads both as one script with the expression inside. Fix: check the parsed step.run values, for example in the PyYAML contract test. (high)
    finding: minor .github/scripts/fork-contracts.test.py:322 The swallow check works by blacklist, and it misses a bare exit line before the guard call. Tested: putting exit above bash .github/scripts/fork-follow-guard.sh in fork-follow.yml's guard step leaves both fork-contracts.test.py and fork-follow.test.sh at "all passed". The step then exits 0 and never runs the guard. Fix: require the refusal step's whole run to be exactly the script call (lines == calls) instead of blacklisting patterns. (high)
    finding: minor .github/workflows/fork-follow.yml:177 (and every copy of the shared step, e.g. fork-release.yml:119) git fetch origin "$REF" with a branch name tries refs/tags/ before refs/heads/. actions/checkout tried the branch first. Tested locally: with a tag main on an older commit, fetching main returns the older commit. So a tag named like FORK_SERIES_BRANCH, or like a fork-release ref input, makes plan or preflight build that tag's commit. Creating such a tag needs write access, and the repo has no tags today. Fix: fetch refs/heads/$REF unless REF is a 40-hex SHA, and add an e2e case. (medium)

@scuttletoby

Copy link
Copy Markdown
Owner Author

lens: security+correctness S-2 · reviewer: opus high (t3lens-12-s2) · read at: 18dadd6
covered: the whole diff against main 8b0ad2c (13 files), plus what it calls: fork-follow-push.sh, fork-follow.sh, fork-release.sh, fork-follow-report.sh, fork-workflows.sh, fork-ci-tests.cjs, the fork_ci_result job, and the upstream workflows in the tree that a PR, label or workflow_run can start (thread-transfer-report, mobile-fingerprint-label, mobile-eas-preview-fork, desktop-macos-preview). Read-only repo state: no rulesets, main not protected, fork-PR approval = all_external_contributors, fork-tests.yml disabled_manually, fork-checks-proof.yml active but not in main's tree (so it does nothing for pull_request_target).
checks:

  • At the head, every fork test passes with GH_TOKEN/GITHUB_TOKEN unset and GH_HOST=bogus.invalid: fork-contracts.test.py, fork-release-workflow.test.py, fork-follow.test.sh, fork-release.test.sh and fork-secret-scan.test.sh all pass; fork-schema-check.test.cjs 51/51; fork-ci-tests.test.cjs 8/8.
  • Red before: fork-contracts.test.py against main's files gives 10 FAIL (pr-guard, the checkout rules, the checkout e2e, gh-guard). Item 2: main's shape test passes with ${{ github.event.pull_request.title }} on the 2nd line of a run block (only the e2e fails, by chance, with "bad substitution"); the head's test fails it. Item 3: main gives 0 fails with GH_TOKEN: ${{ github.token }} in an env; the head gives 1. Item 5: main's fork-follow.test.sh says "all passed" with "|| true" on the guard step; the head's contract test FAILs.
  • Breaking each fix by hand at the head turns it red: taking the guard off no_schema_changes, deleting the guard line from fork-workflows.sh, and putting actions/checkout back into publish.
  • Checkout: the old fetch-depth: 0 fetched every branch and tag; the new step fetches only REF. Every script that uses tags fetches them itself. advance pushes $prev, the old series head, which is not an ancestor of the rebased candidate, so it is not in the checkout. I reproduced this locally: the promisor origin lazy-fetches it, and the backup-tag push succeeds. No script needs a branch checked out (HEAD is detached).
  • State: a re-run of a job gets a fresh workspace. Running the step again in the same workspace fails at "git remote add" (e2e). An empty REF, an unknown ref and a bad HISTORY all fail. A re-run of a run from before the guard uses that run's old workflow file, so fork-checks would check an outside PR again. That is harmless by its design and needs a writer to click it. This PR does not change half-done publish behaviour.
  • CI: fork-tests.yml is disabled, so nothing ran. Once it is enabled, the "Workflow lint and upstream drift" job (fork-ci-tests.cjs runs every listed test, fork-contracts.test.py included, with PyYAML installed) would have run all of the above. actionlint and shellcheck are not installed here, so I did not re-run them.
    finding: major .github/workflows/fork-tests.yml:859 (and fork-checks.yml:24): on an outside PR every job is skipped, fork_ci_result and no_schema_changes included. A job skipped by its if: reports success to branch protection, as this file's own comment at :855-856 says. Once the ruleset this file recommends ("Require only fork-ci-result") exists, an outside PR can merge with a green required check and nothing run. no_schema_changes was safe to run on outside PRs (it reads PR objects only), and now it passes by being skipped. Fix: keep one job that runs only for outside PRs, checks nothing out and exits 1 ("outside PR: push the branch here to test"), with a contract exception for exactly that shape. The PR body names this as a cost; it is the owner's call. Latent today: there is no ruleset. (high)
    finding: minor .github/workflows/fork-tests.yml:55-58: the comment says an outside PR "runs nothing here, not even after approval". Under pull_request the workflow file comes from the PR's merge commit, so a hostile PR deletes the guard, or adds its own workflow, and its code runs once approved. The guard stops only honest PRs. The real controls are the approval setting and the read-only token. Only fork-checks.yml (pull_request_target) gets a guard that the base branch enforces. Reword it so an approver knows that approving still runs the PR's own workflow edits. (high)
    finding: minor .github/scripts/fork-contracts.test.py:313-351: the refusals rule misses three ways to swallow the guard step; with each one fork-contracts.test.py still says "all passed". (1) A bare "exit" line before the guard call: the step exits 0 and the guard never runs. (2) "bash() { echo skip; }" before the call. (3) An earlier step writing BASH_ENV to "$GITHUB_ENV". The PR says it "catches every way". Fix: require the run to be exactly the call (plus the allowed mkdir), and ban GITHUB_ENV/GITHUB_PATH writes in fork-follow.yml. (high)
    finding: minor .github/scripts/fork-contracts.test.py:398-402: the gh-guard rule misses two cases (verified, both pass). A call split with a line continuation ("gh " then " api user") is not seen as a call. A guard line at column 0 inside "if false; then ... fi" counts as sourced. (high)
    finding: minor .github/workflows/thread-transfer-report.yml:4-6 (outside the diff, PR body table): "no fork workflow is named CI" is not enough. An outside PR can add its own workflow with name: CI. After approval, its completion starts this workflow from main with pull-requests: write. The file is not registered, so the sweep cannot disable it before that first run. It runs only default-branch code and reads the artifact as data, so the impact is limited to the text of a PR comment. mobile-fingerprint-label.yml ("Mobile Fingerprint Check") has the same shape. Deleting both in the fork patch closes this. (medium)

…fetch)

- An outside pull request now fails: a new outside_pr job (fork-tests.yml and
  fork-checks.yml) checks nothing out, has permissions {} and exits 1.
  fork_ci_result runs with always(), needs outside_pr and refuses an outside
  pull request first. A job skipped by its if: counts as passed, so before this
  a required fork-ci-result went green on an untested outside pull request.
- fork-contracts.test.py, refusals: a refusal step may hold only the report
  folder and its script call (a whitelist). Earlier steps in its job may be only
  the shared checkout, another refusal or one step-output line. Nothing may write
  GITHUB_ENV, GITHUB_PATH or BASH_ENV, define a function or use an action. No
  BASH_FUNC_* env.
- gh-guard: backslash continuations are joined before matching. The guard line
  must stand at the top level, after nothing but #!, set and comments.
- fork-schema-check.test.cjs: runScripts reads `run:` alone with the script on
  the next line, and plain scalars continued over later lines.
- The shared checkout fetches refs/heads/<REF> unless REF is a 40-hex SHA, so a
  tag named like the branch is never taken. e2e: a tag `main` on an older commit.
- Comments: fork-tests.yml says the guard stops only honest pull requests (the
  PR's own copy runs under pull_request; the approval setting protects). The
  shared step's comment no longer claims fork-tests.yml uses it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@scuttletoby

Copy link
Copy Markdown
Owner Author

not a lens: fix round r2 · head e76f0a3 (was 18dadd6)

This answers the findings in S-1 (#issuecomment-6826888015) and S-2 (#issuecomment-6826894827). The PR body has a new section, "Fix round r2", with its state-question table and a "Not done" list.

# Change Red on 18dadd6 Green on e76f0a3
1 New outside_pr job in fork-tests.yml and fork-checks.yml: if is the guard's exact negation, no checkout, permissions: {}, no secrets, one step, exit 1 with "outside PR: not tested here; a maintainer must review". fork_ci_result now uses always(), needs it, refuses on OUTSIDE_PR first, and lets outside_pr pass only as skipped new contracts test on the old files: 15 pr-guard FAILs, among them the job shape, fork-checks.yml without outside_pr, and fork_ci_result's script: an outside pull request (outside_pr failed, the rest skipped) fails all pass; the script runs 5 result sets (outside with the rest skipped, outside with the rest passed, outside_pr failed on own PR, own PR, push)
2 Refusal steps whitelisted: keys id/name/env/run; run = optional mkdir -p "$RUNNER_TEMP/reports" + one script call; earlier steps in the job = shared checkout, another refusal, or one >> "$GITHUB_OUTPUT" line; no action, GITHUB_ENV/GITHUB_PATH/BASH_ENV write, function, or BASH_FUNC_* env. gh-guard: the guard line must stand at the top level (only #!, set, comments before it) old contracts test and old fork-follow.test.sh both "all passed" with: a bare exit before the guard call; bash() { echo skip; } before it; an earlier step echo "BASH_ENV=/tmp/skip" >> "$GITHUB_ENV". Old contracts "all passed" with fork-workflows.sh's guard inside if false; then … fi. The old test already caught the guard call inside if false; then … fi; the new one still does each FAILs (refusals: no refusal … can be swallowed / gh-guard: … sources fork-gh-guard.sh first); 12 new swallow mutants on each of the 3 refusal steps
3 runScripts takes every line indented past the run: key, whatever the value old "never runs pull request code" passes on fork-checks.yml with run: alone + echo ${{ github.event.pull_request.title }} && node … on the next line, and with a plain run continued by ${{ github.head_ref }} (PyYAML reads both as one script with the expression) that test FAILs on both; 2 new mutants plus runScripts unit cases; 51/51
4 gh calls: \ continuations joined before matching old contracts "all passed" with a new fork-new.sh holding x="$(gh \ / api user)" and no guard FAIL gh-guard: every fork script that calls gh sources fork-gh-guard.sh first
5 Shared checkout fetches refs/heads/$REF unless REF is 40-hex; all 8 copies new e2e on the old step, with an origin tag main on the older commit: FAIL plan and preflight e2e (wrong commit), a branch name takes the branch, never a tag … (tip) and (full), a tag's full ref fails the step all pass
6 fork-tests.yml comment: the guard stops only honest PRs; under pull_request the PR's own copy runs, so the approval setting is the protection (keep it on, never approve outside fork-tests runs). The shared step's comment now says fork-follow.yml and fork-release.yml use it and fork-tests.yml keeps actions/checkout. I didn't move them: empty ref falling back to the event commit, fetch-depth: 0/2, own sparse sets, and step-for-step parity with upstream ci.yml. fork-checks.yml's comment says that ruleset should require "Outside pull request" too comment only, no test n/a

Tests at e76f0a3, in a scratch clone, with GH_TOKEN/GITHUB_TOKEN unset and GH_HOST=bogus.invalid: fork-contracts.test.py (89 ok, all passed), fork-release-workflow.test.py, fork-follow.test.sh, fork-release.test.sh, fork-secret-scan.test.sh all pass. fork-schema-check.test.cjs 51/51, fork-ci-tests.test.cjs 8/8, fork-test-shards.test.cjs 4/4, fork-tests-drift.test.cjs 6/6, and fork-tests-drift.cjs: "match". actionlint 1.7.12 with shellcheck 0.10.0 is clean on fork-*.yml. oxfmt is clean on the changed .cjs and .yml.

Not done (owner's call, in the PR body): deleting thread-transfer-report.yml and mobile-fingerprint-label.yml.

@scuttletoby

Copy link
Copy Markdown
Owner Author

lens: correctness+security r3 · reviewer: opus high (t3lens-12-r3) · read at: e76f0a3
covered: the whole fix diff 18dadd6..e76f0a3 (fork-checks.yml, fork-tests.yml, the 8 copies of the shared checkout in fork-follow.yml and fork-release.yml, fork-contracts.test.py, fork-schema-check.test.cjs), plus what it calls into: fork_ci_result's whole script, fork-follow.yml's guard and advance jobs, every REF source of the shared step (github.sha, SERIES_BRANCH, plan's candidate, inputs.ref, preflight's ref = fork-release.sh's "ref=$head"), and the S-1 and S-2 comments with the r2 reply.
checks:

  • Tests at e76f0a3 in a scratch clone, with GH_TOKEN and GITHUB_TOKEN unset and GH_HOST=bogus.invalid: fork-contracts.test.py "all passed"; fork-schema-check.test.cjs 51/51.
  • Red before. The new contracts test on 18dadd6's files gives 20 FAILs: 15 pr-guard (among them the outside_pr shape, "fork-checks.yml without outside_pr" and all 5 fork_ci_result script cases) and 5 checkout e2e (plan and preflight take the tag main, both tag cases, and the refs/tags/main case). Items 2 and 4 are fixes to the test itself, so I ran the OLD contracts test on mutated files instead. A bare exit before the guard call: 0 FAIL. bash() { echo skip; } before it: 0 FAIL. A new fork-new.sh with gh \ / api user and no guard: 0 FAIL. Item 3: the new schema test with the old runScripts put back fails 2 (the unit case and the next-line mutant).
  • Breaking each fix by hand at the head turns it red. The bare exit and bash() mutants fail the refusals check; the split gh fails gh-guard. With refs/heads/ reverted in the first copy of fork-follow.yml, the checkout rule and the plan and preflight e2e cases fail. With it reverted in the first copy of fork-release.yml, the checkout rule fails. Deleting fork_ci_result's OUTSIDE_PR block fails pr-guard plus 2 script cases. Letting outside_pr pass with any result in the jq fails "outside_pr failed on a run that is no outside pull request". Deleting outside_pr from fork-checks.yml fails 2 checks.
  • Outside-PR paths. outside_pr checks nothing out, runs one fixed echo plus exit 1, has permissions: {} and interpolates nothing. fork_ci_result reads needs and OUTSIDE_PR only through env, and refuses before any later step. Its other steps are only download-artifact (the contract test pins that). Its if: is always(), and outside_pr may pass only as "skipped". So on an outside PR, an outside_pr that was skipped, cancelled or failed cannot give a green fork-ci-result, because the OUTSIDE_PR block exits 1 first, whatever needs says. A deleted head repo makes OUTSIDE_IF true (null != repo): it fails, never skips. On a push, OUTSIDE_PR is "" (tested). Under workflow_call, the caller's event has no pull_request.
  • Checkout. A 40-hex REF is fetched as is; anything else goes through refs/heads/. Every real REF is a 40-hex SHA or a branch (fork-release.sh writes ref=$head as a SHA). A tag, refs/heads/x, an empty value or a short SHA fails the step, and a short SHA failed before too. This fails closed. The only behaviour change is that a workflow_dispatch of fork-release with a tag name now fails, which matches its "Commit or branch" description.
  • actionlint 1.7.7 (an older copy found locally; shellcheck is not installed) on fork-*.yml gives only "label macos-26 is unknown" at fork-tests.yml:820. That line is outside this diff, and the label is too new for 1.7.7.
  • State question. (a) An outside PR whose head has only pre-fix runs has no "Outside pull request" check at all. A ruleset that requires it then stays blocked, which fails closed. (b) Re-running an old pre-fix run uses that run's workflow file, so it can only add skipped (green) old-style jobs for that SHA. Any new event (synchronize, edited, reopened) runs main's fork-checks.yml, and its failing outside_pr stays on the head SHA. (c) A PR from a branch of this fork: head.repo == repository, so outside_pr is skipped and fork_ci_result accepts "skipped" for it. A PR from upstream or another fork: outside. (d) A branch and a tag with the same name: the branch wins in both history modes (e2e). An empty REF fails. (e) With fork-tests.yml still disabled, fork-ci-result never reports. If it is required, every PR is blocked (fails closed). Today only fork-checks' outside_pr refuses outside PRs. (f) Half-applied: these jobs write nothing, and a re-run of a job gets a fresh workspace.
    finding: minor .github/scripts/fork-contracts.test.py:495 The earlier-step rule accepts any step for which is_refusal() is true, and is_refusal() is only a substring test: "fork-follow-guard.sh" anywhere in run, or env REQUIRE_NEW_OFF: "true". refusal_steps() whitelists only steps containing "bash .github/scripts/". So a step before the guard with run: echo 'exit 0' > .github/scripts/fork-follow-guard.sh passes the test. So does a step with env REQUIRE_NEW_OFF "true" and any run, e.g. one that rewrites .github/scripts. Tested: the contracts test says "all passed" for both, while the control (the same step without the name) FAILs. Then the guard runs a script that exits 0. Only a deliberate change gets here, which is why it is minor. Fix: count an earlier step as a refusal only if it passes the refusal whitelist itself (its body is SCRIPT_LINE, optionally after the mkdir), not on a substring. (high)
    finding: minor .github/workflows/fork-checks.yml:60 (and fork-tests.yml:71) Both workflows name their job "Outside pull request", and fork-checks.yml:22 tells the ruleset to require that name. fork-tests.yml runs the PR's own copy under pull_request. Once a maintainer approves an outside run, that copy can report a passing check of the same name next to the failing one from main's fork-checks.yml. Required checks match by name, so the one check an outside PR cannot write itself becomes ambiguous. This needs a deliberate PR and an approval the fork-tests comment already forbids. Fix: give fork-checks' job a distinct name, e.g. "Outside pull request (checks)", and name exactly that in the comment. (medium: I did not test how GitHub resolves two required checks with the same name)

@scuttletoby
scuttletoby merged commit 919556a into main Nov 12, 2026
0 of 2 checks passed
scuttletoby added a commit that referenced this pull request Nov 12, 2026
Conflicts in fork-tests.yml: each routed job keeps #13's needs and runs-on and
takes main's pull request guard in its if:. The new `route` job gets the guard
too (fork-contracts.test.py: every job a pull request starts begins with it).
fork-ci-tests.txt lists both new tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant