From 7f9c5df7db807ca755bbacc1e15f5765b98934eb Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:00:40 +0530 Subject: [PATCH 01/13] fix(ci): close shell injection into the npm publish job A refname is attacker-controlled and git permits backtick, $, (, ; and | in it. Three sites substituted it into a run: block, and NODE_AUTH_TOKEN sat at job level, so a pushed tag ran arbitrary commands with the publish credential in reach. The tag now goes through env:, is validated against an anchored version pattern before anything consumes it, and reaches the other jobs as a job output. The token is scoped to the publish step. release-npm declares contents: read instead of inheriting the repo default. --- .github/workflows/release.yml | 61 ++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 93f2c61..5e35c1d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,26 +11,55 @@ on: required: true type: string +permissions: + contents: read + concurrency: group: release-${{ github.ref }} cancel-in-progress: false jobs: + resolve-tag: + name: Resolve and validate the tag + runs-on: ubuntu-latest + permissions: {} + outputs: + tag: ${{ steps.tag.outputs.tag }} + version: ${{ steps.tag.outputs.version }} + steps: + # A refname is attacker-controlled text and git permits backtick, `$`, + # `(`, `;`, `&` and `|` in it, so it goes through env: a `${{ }}` is + # substituted before bash ever sees the line. Every later job reads these + # outputs rather than the refname, and nothing reaches a shell before it + # has matched the pattern. The pattern is anchored and admits no newline, + # which is what stops the value below forging a second $GITHUB_OUTPUT key. + - name: Validate the tag + id: tag + run: | + pattern='^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z]+(\.[0-9A-Za-z]+)*)?$' + if [[ ! "$TAG" =~ $pattern ]]; then + echo "release: refusing to publish from '$TAG'" >&2 + echo "release: a release tag is vMAJOR.MINOR.PATCH with an optional -prerelease, e.g. v0.1.0 or v0.0.1-rc1" >&2 + exit 1 + fi + echo "tag=$TAG" >> "$GITHUB_OUTPUT" + echo "version=${TAG#v}" >> "$GITHUB_OUTPUT" + env: + TAG: ${{ inputs.tag || github.ref_name }} + release-npm: name: Publish @sanderling/spec to npm + needs: resolve-tag runs-on: ubuntu-latest - env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + permissions: + contents: read steps: - uses: actions/checkout@v7 with: - ref: ${{ inputs.tag || github.ref }} - - - name: Resolve version - id: ver - run: | - raw="${{ inputs.tag || github.ref_name }}" - echo "version=${raw#v}" >> "$GITHUB_OUTPUT" + ref: ${{ needs.resolve-tag.outputs.tag }} + # `npm ci` below runs dependency lifecycle scripts, and no step in + # this job needs the git credential afterwards. + persist-credentials: false - name: Set up Node 22 uses: actions/setup-node@v7 @@ -46,28 +75,36 @@ jobs: - name: Stamp version working-directory: pkg/spec - run: npm version ${{ steps.ver.outputs.version }} --no-git-tag-version --allow-same-version + run: npm version "$VERSION" --no-git-tag-version --allow-same-version + env: + VERSION: ${{ needs.resolve-tag.outputs.version }} - name: Publish working-directory: pkg/spec # npm tag pre-releases (e.g. 0.1.0-rc1) as "next" so npm install @sanderling/spec # keeps resolving the latest stable. run: | - if [[ "${{ steps.ver.outputs.version }}" == *-* ]]; then + if [[ "$VERSION" == *-* ]]; then npm publish --access public --tag next else npm publish --access public fi + # The publish credential is scoped to the one step that publishes rather + # than to the job, so no other step runs with it in reach. + env: + VERSION: ${{ needs.resolve-tag.outputs.version }} + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} release-cli: name: Publish sanderling CLI to GitHub Releases + needs: resolve-tag runs-on: ubuntu-latest permissions: contents: write steps: - uses: actions/checkout@v7 with: - ref: ${{ inputs.tag || github.ref }} + ref: ${{ needs.resolve-tag.outputs.tag }} fetch-depth: 0 - name: Set up Go From 6263aa4c6c0ae04a8415a4324bb45f089f5cdca2 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:08:27 +0530 Subject: [PATCH 02/13] fix(ci): a run that wrote no trace is not evidence about folio run_dir is empty when the run produced no output directory, and the fallback made trace ./trace.jsonl. A stray trace in the working directory was then read as this run's, so a run that wrote nothing reported 'found the submit bug' and exited 0, defeating the missing-trace check below it. --- .github/scripts/folio-run.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/scripts/folio-run.sh b/.github/scripts/folio-run.sh index d4c526d..0977f91 100755 --- a/.github/scripts/folio-run.sh +++ b/.github/scripts/folio-run.sh @@ -89,7 +89,11 @@ esac code=$? run_dir="$(ls -d "$output"/*/ 2>/dev/null | tail -1)" -trace="${run_dir:-.}/trace.jsonl" +# No run directory means no trace. Defaulting the directory to `.` here reads a +# stray ./trace.jsonl and reports it as this run's evidence, which is how a run +# that wrote nothing at all reached "found the submit bug" and exit 0. +trace="" +[ -n "$run_dir" ] && trace="${run_dir}trace.jsonl" steps=0 [ -f "$trace" ] && steps=$(wc -l < "$trace" | tr -d ' ') From cd3bc51a0d91e7848477826f87c3be9168986783 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:08:40 +0530 Subject: [PATCH 03/13] fix(ci): fail folio when a gated property is not in the spec Nothing tied GATED_PROPERTIES to the spec it gates. Renaming a property left the classifier matching nothing: ios and web blamed the spec for finding a different bug, and android silently reclassified a real conviction as 'judging health only' and stayed green. replay-ui-summary.sh already makes this check for its own list. The spec path becomes SPEC-overridable the same way, so the check is testable. --- .github/scripts/folio-run.sh | 46 ++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/.github/scripts/folio-run.sh b/.github/scripts/folio-run.sh index 0977f91..fb3d929 100755 --- a/.github/scripts/folio-run.sh +++ b/.github/scripts/folio-run.sh @@ -18,9 +18,49 @@ max_steps="${MAX_STEPS:-240}" duration="${DURATION:-20m}" sanderling="${SANDERLING:-./bin/sanderling}" output="runs/folio-$platform" -spec="examples/folio/sanderling/spec.ts" +spec="${SPEC:-examples/folio/sanderling/spec.ts}" summary="${GITHUB_STEP_SUMMARY:-/dev/null}" +# The two properties that state folio's double-submit. Anything else the spec +# proves false is a different finding, and this leg has nothing to say about it. +GATED_PROPERTIES="submitMovesBalanceByAtMostTypedAmount,submitCommitsOneTransactionPerAction" + +# A gate is only as good as these names, and nothing else ties them to the spec. +# Rename a property there and the classification below matches nothing: ios and +# web blame the spec for finding a different bug, and android reclassifies a +# real conviction as "judging health only" and stays green. Checked before the +# run so a rename costs seconds rather than the whole budget. +SPEC="$spec" GATED="$GATED_PROPERTIES" SELF="$0" python3 - <<'PY' || exit 1 +import os, re, sys + +spec_path = os.environ["SPEC"] +gated = [name for name in os.environ["GATED"].split(",") if name] +try: + with open(spec_path, encoding="utf-8") as handle: + source = handle.read() +except OSError as error: + sys.exit("folio: cannot read %s to check the gated properties still exist: %s" + % (spec_path, error)) + +block = re.search(r"export\s+const\s+properties\s*=\s*\{(.*?)\}", source, re.S) +if block is None: + sys.exit("folio: %s declares no `export const properties = {...}`, so the gated " + "properties cannot be checked against it" % spec_path) + +declared = set() +for entry in re.sub(r"//[^\n]*", "", block.group(1)).split(","): + name = entry.split(":")[0].strip() + if re.fullmatch(r"[A-Za-z_$][A-Za-z0-9_$]*", name): + declared.add(name) + +missing = [name for name in gated if name not in declared] +if missing: + sys.exit("folio: %s no longer declares %s, so this leg gates on a property that " + "cannot be violated and every real conviction would read as a different " + "finding. Update GATED_PROPERTIES in %s." + % (spec_path, ", ".join(missing), os.environ["SELF"])) +PY + folio_args=(--bundle-id app.folio) case "$platform" in android) @@ -97,10 +137,6 @@ trace="" steps=0 [ -f "$trace" ] && steps=$(wc -l < "$trace" | tr -d ' ') -# The two properties that state folio's double-submit. Anything else the spec -# proves false is a different finding, and this leg has nothing to say about it. -GATED_PROPERTIES="submitMovesBalanceByAtMostTypedAmount,submitCommitsOneTransactionPerAction" - # Exit 2 means "the run recorded a violation", and that is NOT the same as "the # run convicted folio". A predicate that THROWS is recorded as a violation too, # with is_error set and the thrown text as its reason, and it reaches exit 2 by From 41c6c1e4b930c5fad4d4ba9428643f2adf1b88b0 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:08:45 +0530 Subject: [PATCH 04/13] test(ci): cover the folio classifier's verdicts 21 cases through a stubbed sanderling: every exit path, the drift check, a missing trace, a zero-byte trace, an empty glob and a truncated line. Asserts the flags that reached the binary, not just the exit code. Invoked as bash -eo pipefail -c, which is what a run: block does. Running folio-run.sh itself under -e would kill it at the first non-zero sanderling test, which is the exit code it exists to read. --- .github/scripts/folio-run-test.sh | 284 ++++++++++++++++++++++++++++++ 1 file changed, 284 insertions(+) create mode 100755 .github/scripts/folio-run-test.sh diff --git a/.github/scripts/folio-run-test.sh b/.github/scripts/folio-run-test.sh new file mode 100755 index 0000000..dc9757a --- /dev/null +++ b/.github/scripts/folio-run-test.sh @@ -0,0 +1,284 @@ +#!/usr/bin/env bash +# Drives folio-run.sh through a stubbed `sanderling` binary and checks the +# verdict it reaches from each shape of trace. What is under test is the +# classification, not the fuzzer: the stub writes the trace the run would have +# written and exits the code the run would have exited. +# +# folio-run.sh is invoked as `bash -eo pipefail -c