From 2c82c366c668123ca63f27c29350213c2c047354 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 21:52:46 +0900 Subject: [PATCH 1/6] fix(osv): keep base scan results across fork checkout Copy a non-empty old-results.json into RUNNER_TEMP before the head checkout and restore it before compare, including scanner output under source/, so a zero-finding fork PR cannot fail on an empty base file. --- .github/workflows/security-scan.yml | 32 +++++++++++++++++++ CHANGELOG.md | 4 +++ .../osv-cross-fork-result-isolation.md | 11 ++++--- .../test_required_workflow_queue_contract.py | 6 +++- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 29376269d..bf38c20a1 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -105,6 +105,19 @@ jobs: --allow-no-lockfiles -r source/ + - name: Preserve base OSV output outside the checkout path + run: | + set -euo pipefail + dest="${RUNNER_TEMP}/osv-old-results.json" + for candidate in old-results.json source/old-results.json; do + if [ -s "${candidate}" ]; then + cp "${candidate}" "${dest}" + cp "${dest}" "${GITHUB_WORKSPACE}/old-results.json" + echo "Preserved OSV base output from ${candidate}" + exit 0 + fi + done + echo "::warning::OSV base output was not present after the base scan steps." - name: Checkout head uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -113,6 +126,16 @@ jobs: path: source fetch-depth: 0 persist-credentials: false + - name: Restore preserved base OSV output + run: | + set -euo pipefail + src="${RUNNER_TEMP}/osv-old-results.json" + if [ -s "${src}" ]; then + cp "${src}" "${GITHUB_WORKSPACE}/old-results.json" + fi + if [ ! -s old-results.json ] && [ -s source/old-results.json ]; then + cp source/old-results.json old-results.json + fi - name: Scan head with OSV id: osv_head continue-on-error: true @@ -147,6 +170,15 @@ jobs: - name: Require OSV scan output run: | set -euo pipefail + if [ ! -s old-results.json ] && [ -s source/old-results.json ]; then + cp source/old-results.json old-results.json + fi + if [ ! -s new-results.json ] && [ -s source/new-results.json ]; then + cp source/new-results.json new-results.json + fi + if [ ! -s old-results.json ] && [ -s "${RUNNER_TEMP}/osv-old-results.json" ]; then + cp "${RUNNER_TEMP}/osv-old-results.json" old-results.json + fi test -s old-results.json test -s new-results.json - name: Print OSV findings being compared diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b0ef8d44..cd532a0b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Preserve OSV `old-results.json` in `RUNNER_TEMP` before a fork head checkout + and restore it before compare, including when the scanner writes the file + under `source/`, so a zero-finding fork scan cannot fail on an empty base + artifact. - Honor each trusted base project's exact, integrity-bearing pnpm `packageManager` specification in OpenCode coverage images through the pinned Node distribution's Corepack runtime, instead of admitting the specification diff --git a/docs/doctoring/osv-cross-fork-result-isolation.md b/docs/doctoring/osv-cross-fork-result-isolation.md index c9a6c0a98..a122bc118 100644 --- a/docs/doctoring/osv-cross-fork-result-isolation.md +++ b/docs/doctoring/osv-cross-fork-result-isolation.md @@ -13,11 +13,12 @@ whose repository identity changed. ## Decision Checkout both exact repositories into the same `source/` child directory and -scan that directory. The head checkout may replace `source/` when the repository -identity changes, but the base result remains at the workspace root. Reusing the -same checkout path also gives the base and head scan identical source paths, so -the existing OSV comparison retains its meaning. Missing or empty output remains -a hard failure; this change does not weaken vulnerability comparison. +scan that directory. Immediately copy a non-empty base result from the +workspace root or `source/old-results.json` into `${RUNNER_TEMP}/osv-old-results.json`, +then restore that file after the fork head checkout. Reusing the same checkout +path keeps base and head scan source paths comparable. Missing or empty output +after restore remains a hard failure; a zero-finding head scan does not skip +the base comparison. This change does not weaken vulnerability comparison. ## Verification and rollback diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index e58f5e6c0..fb432cc11 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1264,8 +1264,12 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non assert workflow.count("path: source") == 2 assert workflow.count("--output=old-results.json") == 2 assert workflow.count("--output=new-results.json") == 2 - assert workflow.count("source/") == 4 + assert workflow.count("-r\n source/") == 4 assert "clean: false" not in workflow + assert "Preserve base OSV output outside the checkout path" in workflow + assert "Restore preserved base OSV output" in workflow + assert "${RUNNER_TEMP}/osv-old-results.json" in workflow + assert "source/old-results.json" in workflow assert "test -s old-results.json" in workflow assert "test -s new-results.json" in workflow From 96ef882e20faac2996094b7cc6ead99de6c164ae Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 22:05:51 +0900 Subject: [PATCH 2/6] fix(osv): restore captured scan output without root overwrite Copy OSV results only into RUNNER_TEMP, unlink any root-owned workspace copy before restore, and discard checkout-provided JSON before each scan so a fork cannot plant reporter input. Observed quality failure was permission denied while overwriting old-results.json. --- .github/workflows/security-scan.yml | 52 ++++++++++++------- CHANGELOG.md | 9 ++-- .../osv-cross-fork-result-isolation.md | 29 +++++++---- .../test_required_workflow_queue_contract.py | 30 ++++++++++- 4 files changed, 85 insertions(+), 35 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index bf38c20a1..1971f116e 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -74,6 +74,10 @@ jobs: path: source fetch-depth: 0 persist-credentials: false + - name: Discard checkout-provided OSV result files + run: | + set -euo pipefail + rm -f source/old-results.json source/new-results.json - name: Scan base with OSV id: osv_base continue-on-error: true @@ -112,12 +116,12 @@ jobs: for candidate in old-results.json source/old-results.json; do if [ -s "${candidate}" ]; then cp "${candidate}" "${dest}" - cp "${dest}" "${GITHUB_WORKSPACE}/old-results.json" echo "Preserved OSV base output from ${candidate}" exit 0 fi done - echo "::warning::OSV base output was not present after the base scan steps." + echo "::error::OSV base output was not present after the base scan steps." + exit 1 - name: Checkout head uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -130,12 +134,13 @@ jobs: run: | set -euo pipefail src="${RUNNER_TEMP}/osv-old-results.json" - if [ -s "${src}" ]; then - cp "${src}" "${GITHUB_WORKSPACE}/old-results.json" - fi - if [ ! -s old-results.json ] && [ -s source/old-results.json ]; then - cp source/old-results.json old-results.json - fi + test -s "${src}" + rm -f "${GITHUB_WORKSPACE}/old-results.json" + cp "${src}" "${GITHUB_WORKSPACE}/old-results.json" + - name: Discard checkout-provided OSV result files + run: | + set -euo pipefail + rm -f source/old-results.json source/new-results.json - name: Scan head with OSV id: osv_head continue-on-error: true @@ -167,20 +172,29 @@ jobs: --allow-no-lockfiles -r source/ + - name: Capture head OSV output outside the checkout path + run: | + set -euo pipefail + dest="${RUNNER_TEMP}/osv-new-results.json" + for candidate in new-results.json source/new-results.json; do + if [ -s "${candidate}" ]; then + cp "${candidate}" "${dest}" + echo "Captured OSV head output from ${candidate}" + exit 0 + fi + done + echo "::error::OSV head output was not present after the head scan steps." + exit 1 - name: Require OSV scan output run: | set -euo pipefail - if [ ! -s old-results.json ] && [ -s source/old-results.json ]; then - cp source/old-results.json old-results.json - fi - if [ ! -s new-results.json ] && [ -s source/new-results.json ]; then - cp source/new-results.json new-results.json - fi - if [ ! -s old-results.json ] && [ -s "${RUNNER_TEMP}/osv-old-results.json" ]; then - cp "${RUNNER_TEMP}/osv-old-results.json" old-results.json - fi - test -s old-results.json - test -s new-results.json + old="${RUNNER_TEMP}/osv-old-results.json" + new="${RUNNER_TEMP}/osv-new-results.json" + test -s "${old}" + test -s "${new}" + rm -f old-results.json new-results.json + cp "${old}" old-results.json + cp "${new}" new-results.json - name: Print OSV findings being compared shell: python3 {0} run: | diff --git a/CHANGELOG.md b/CHANGELOG.md index cd532a0b7..f5b8649ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,11 @@ Semantic Versioning where the repository publishes a release. ## [Unreleased] -- Preserve OSV `old-results.json` in `RUNNER_TEMP` before a fork head checkout - and restore it before compare, including when the scanner writes the file - under `source/`, so a zero-finding fork scan cannot fail on an empty base - artifact. +- Preserve OSV scan output in `RUNNER_TEMP` before a fork head checkout and + restore it by unlinking any root-owned workspace copy first, so a + permission-denied overwrite cannot drop the base artifact; discard + checkout-provided result files before each scan and never treat post-checkout + `source/*.json` as reporter input. - Honor each trusted base project's exact, integrity-bearing pnpm `packageManager` specification in OpenCode coverage images through the pinned Node distribution's Corepack runtime, instead of admitting the specification diff --git a/docs/doctoring/osv-cross-fork-result-isolation.md b/docs/doctoring/osv-cross-fork-result-isolation.md index a122bc118..fcbe31ca6 100644 --- a/docs/doctoring/osv-cross-fork-result-isolation.md +++ b/docs/doctoring/osv-cross-fork-result-isolation.md @@ -8,25 +8,34 @@ checkout used the upstream repository while the head belonged to a fork. `actions/checkout` detected the different repository origin and replaced the workspace before checking out the fork, including the untracked base result. `clean: false` does not preserve files when checkout must replace a workspace -whose repository identity changed. +whose repository identity changed. Copying a captured result back onto the +workspace `old-results.json` can also fail: the scanner action may create that +file as root, so a runner-user overwrite returns permission denied. ## Decision Checkout both exact repositories into the same `source/` child directory and -scan that directory. Immediately copy a non-empty base result from the -workspace root or `source/old-results.json` into `${RUNNER_TEMP}/osv-old-results.json`, -then restore that file after the fork head checkout. Reusing the same checkout -path keeps base and head scan source paths comparable. Missing or empty output -after restore remains a hard failure; a zero-finding head scan does not skip -the base comparison. This change does not weaken vulnerability comparison. +scan that directory. Copy a non-empty scanner result into +`${RUNNER_TEMP}/osv-old-results.json` and `${RUNNER_TEMP}/osv-new-results.json` +immediately after each scan. Do not copy those captures back onto an existing +workspace `old-results.json`: the OSV action may create that file as root, and +a runner-user `cp` then fails with permission denied (observed on +ContextualWisdomLab/.github#1257). Restore by unlinking the workspace file +first, then copying from `RUNNER_TEMP`. Discard `source/old-results.json` and +`source/new-results.json` before each scan so a fork cannot plant reporter +input. After the head checkout, never treat checkout-path JSON as scanner +output. Missing or empty captured output remains a hard failure; a +zero-finding head scan does not skip the base comparison. This change does not +weaken vulnerability comparison. ## Verification and rollback - The workflow contract proves both checkouts target `source/`, every scan reads - that same directory, and both result files remain at the workspace root. + that same directory, captures land in `RUNNER_TEMP` before compare, restore + unlinks before copy, and post-checkout `source/*.json` is not reporter input. - `actionlint` validates the edited workflow. -- Rerun a fork PR's `Security Scan`; both `old-results.json` and - `new-results.json` must be non-empty before the reporter runs. +- Rerun a fork PR's `Security Scan`; both captured result files must be + non-empty before the reporter runs. - Roll back only after another job-scoped store retains the base artifact across repository replacement without changing the compared source paths. diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index fb432cc11..cb88e1e2d 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1268,10 +1268,36 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non assert "clean: false" not in workflow assert "Preserve base OSV output outside the checkout path" in workflow assert "Restore preserved base OSV output" in workflow + assert "Capture head OSV output outside the checkout path" in workflow assert "${RUNNER_TEMP}/osv-old-results.json" in workflow + assert "${RUNNER_TEMP}/osv-new-results.json" in workflow assert "source/old-results.json" in workflow - assert "test -s old-results.json" in workflow - assert "test -s new-results.json" in workflow + assert "source/new-results.json" in workflow + preserve = workflow.index( + " - name: Preserve base OSV output outside the checkout path" + ) + checkout_head = workflow.index(" - name: Checkout head") + restore = workflow.index(" - name: Restore preserved base OSV output") + scan_head = workflow.index(" - name: Scan head with OSV") + capture_head = workflow.index( + " - name: Capture head OSV output outside the checkout path" + ) + require_output = workflow.index(" - name: Require OSV scan output") + assert preserve < checkout_head < restore < scan_head < capture_head < require_output + discard_after_restore = workflow.index( + " - name: Discard checkout-provided OSV result files", restore + ) + restore_block = workflow[restore:discard_after_restore] + require_block = workflow[ + require_output : workflow.index(" - name: Print OSV findings being compared") + ] + assert "source/old-results.json" not in restore_block + assert "source/old-results.json" not in require_block + assert "source/new-results.json" not in require_block + assert 'cp "${dest}" "${GITHUB_WORKSPACE}/old-results.json"' not in workflow + assert "rm -f source/old-results.json source/new-results.json" in workflow + assert 'test -s "${old}"' in require_block + assert 'test -s "${new}"' in require_block def test_secret_scan_push_limits_gitleaks_to_current_branch_history() -> None: From 19a9bc0cb07ebb6a6f0a20c6b39c552d53bb1e8e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 22:37:07 +0900 Subject: [PATCH 3/6] fix(osv): transfer root-owned results safely --- .github/workflows/security-scan.yml | 16 +++- CHANGELOG.md | 10 +-- .../osv-cross-fork-result-isolation.md | 28 +++++-- .../test_required_workflow_queue_contract.py | 76 +++++++++++++++++++ 4 files changed, 116 insertions(+), 14 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 1971f116e..59c377b1b 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -113,9 +113,13 @@ jobs: run: | set -euo pipefail dest="${RUNNER_TEMP}/osv-old-results.json" + rm -f "${dest}" + umask 077 for candidate in old-results.json source/old-results.json; do - if [ -s "${candidate}" ]; then - cp "${candidate}" "${dest}" + if [ -f "${candidate}" ] && [ ! -L "${candidate}" ] && [ -s "${candidate}" ]; then + /usr/bin/sudo --non-interactive /usr/bin/cat -- "${candidate}" | /usr/bin/tee "${dest}" >/dev/null + test -s "${dest}" + test -O "${dest}" echo "Preserved OSV base output from ${candidate}" exit 0 fi @@ -176,9 +180,13 @@ jobs: run: | set -euo pipefail dest="${RUNNER_TEMP}/osv-new-results.json" + rm -f "${dest}" + umask 077 for candidate in new-results.json source/new-results.json; do - if [ -s "${candidate}" ]; then - cp "${candidate}" "${dest}" + if [ -f "${candidate}" ] && [ ! -L "${candidate}" ] && [ -s "${candidate}" ]; then + /usr/bin/sudo --non-interactive /usr/bin/cat -- "${candidate}" | /usr/bin/tee "${dest}" >/dev/null + test -s "${dest}" + test -O "${dest}" echo "Captured OSV head output from ${candidate}" exit 0 fi diff --git a/CHANGELOG.md b/CHANGELOG.md index f5b8649ee..8cbbae699 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,11 @@ Semantic Versioning where the repository publishes a release. ## [Unreleased] -- Preserve OSV scan output in `RUNNER_TEMP` before a fork head checkout and - restore it by unlinking any root-owned workspace copy first, so a - permission-denied overwrite cannot drop the base artifact; discard - checkout-provided result files before each scan and never treat post-checkout - `source/*.json` as reporter input. +- Preserve OSV scan output in private, runner-owned `RUNNER_TEMP` files before a + fork head checkout, using a bounded privileged read so root-owned mode-`0600` + scanner output does not depend on world readability; restore by unlinking any + root-owned workspace copy first, discard checkout-provided result files before + each scan, and never treat post-checkout `source/*.json` as reporter input. - Honor each trusted base project's exact, integrity-bearing pnpm `packageManager` specification in OpenCode coverage images through the pinned Node distribution's Corepack runtime, instead of admitting the specification diff --git a/docs/doctoring/osv-cross-fork-result-isolation.md b/docs/doctoring/osv-cross-fork-result-isolation.md index fcbe31ca6..2f33c0920 100644 --- a/docs/doctoring/osv-cross-fork-result-isolation.md +++ b/docs/doctoring/osv-cross-fork-result-isolation.md @@ -11,6 +11,10 @@ workspace before checking out the fork, including the untracked base result. whose repository identity changed. Copying a captured result back onto the workspace `old-results.json` can also fail: the scanner action may create that file as root, so a runner-user overwrite returns permission denied. +The inverse copy has the same unowned boundary: a runner-user `cp` can read a +root-owned scanner result only while the container happens to leave it +world-readable. A mode such as `0600` makes that capture fail before the +checkout-isolation guarantee can be established. ## Decision @@ -20,19 +24,29 @@ scan that directory. Copy a non-empty scanner result into immediately after each scan. Do not copy those captures back onto an existing workspace `old-results.json`: the OSV action may create that file as root, and a runner-user `cp` then fails with permission denied (observed on -ContextualWisdomLab/.github#1257). Restore by unlinking the workspace file -first, then copying from `RUNNER_TEMP`. Discard `source/old-results.json` and +ContextualWisdomLab/.github#1257). For the scanner-to-runner transfer, accept +only a fixed-path, non-empty regular file that is not a symbolic link, use the +GitHub-hosted Linux runner's passwordless `/usr/bin/sudo` solely for a fixed +`/usr/bin/cat`, and let the unprivileged shell create the destination after +`umask 077`. The resulting `RUNNER_TEMP` capture is therefore runner-owned and +mode `0600`; the workflow verifies both ownership and non-empty content. It +does not `chmod` the scanner output, grant other users read access, or make the +reporter privileged. Restore by unlinking the workspace file first, then +copying from `RUNNER_TEMP`. Discard `source/old-results.json` and `source/new-results.json` before each scan so a fork cannot plant reporter input. After the head checkout, never treat checkout-path JSON as scanner -output. Missing or empty captured output remains a hard failure; a -zero-finding head scan does not skip the base comparison. This change does not -weaken vulnerability comparison. +output. Missing or empty captured output remains a hard failure; a zero-finding +head scan does not skip the base comparison. This change does not weaken +vulnerability comparison. ## Verification and rollback - The workflow contract proves both checkouts target `source/`, every scan reads that same directory, captures land in `RUNNER_TEMP` before compare, restore unlinks before copy, and post-checkout `source/*.json` is not reporter input. +- The executable regression runs both production capture steps against an + unreadable scanner result through a bounded privilege stand-in, then proves + the capture contains the exact result and is runner-owned with mode `0600`. - `actionlint` validates the edited workflow. - Rerun a fork PR's `Security Scan`; both captured result files must be non-empty before the reporter runs. @@ -44,6 +58,10 @@ weaken vulnerability comparison. GitHub. (2026). *Variables reference*. GitHub Docs. Retrieved August 22, 2026, from https://docs.github.com/en/actions/reference/variables-reference +GitHub. (2026). *GitHub-hosted runners reference*. GitHub Docs. Retrieved +August 23, 2026, from +https://docs.github.com/en/actions/reference/runners/github-hosted-runners + GitHub Actions. (2026). *Checkout*. GitHub. Retrieved August 22, 2026, from https://github.com/actions/checkout diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index cb88e1e2d..e59349d3c 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -4,6 +4,7 @@ import os import shlex import shutil +import stat import subprocess import sys import textwrap @@ -1300,6 +1301,81 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non assert 'test -s "${new}"' in require_block +@pytest.mark.parametrize( + ("step_name", "candidate_name", "destination_name"), + [ + ( + "Preserve base OSV output outside the checkout path", + "old-results.json", + "osv-old-results.json", + ), + ( + "Capture head OSV output outside the checkout path", + "source/new-results.json", + "osv-new-results.json", + ), + ], +) +def test_security_scan_transfers_unreadable_scanner_output_to_runner( + tmp_path: Path, + step_name: str, + candidate_name: str, + destination_name: str, +) -> None: + """A privileged read must create a private runner-owned capture.""" + workflow = workflow_text("security-scan.yml") + run_marker = " run: |\n" + step = workflow_step(workflow, step_name) + assert run_marker in step + + fake_sudo = tmp_path / "sudo" + fake_sudo.write_text( + """#!/bin/sh +set -eu +test "$1" = "--non-interactive" +shift +test "$1" = "/usr/bin/cat" +shift +test "$1" = "--" +shift +chmod u+r "$1" +exec /bin/cat "$1" +""", + encoding="utf-8", + ) + fake_sudo.chmod(0o700) + + source = tmp_path / candidate_name + source.parent.mkdir(parents=True, exist_ok=True) + expected = '{"results": []}\n' + source.write_text(expected, encoding="utf-8") + source.chmod(0) + runner_temp = tmp_path / "runner-temp" + runner_temp.mkdir() + + script = textwrap.dedent(step.split(run_marker, 1)[1]).replace( + "/usr/bin/sudo", shlex.quote(str(fake_sudo)) + ) + result = subprocess.run( + ["/bin/bash", "-c", script], + cwd=tmp_path, + env={ + **os.environ, + "GITHUB_WORKSPACE": str(tmp_path), + "RUNNER_TEMP": str(runner_temp), + }, + check=False, + capture_output=True, + text=True, + ) + + destination = runner_temp / destination_name + assert result.returncode == 0, result.stderr + assert destination.read_text(encoding="utf-8") == expected + assert destination.stat().st_uid == os.geteuid() + assert stat.S_IMODE(destination.stat().st_mode) == 0o600 + + def test_secret_scan_push_limits_gitleaks_to_current_branch_history() -> None: """Limit push secret scanning to the current branch history.""" workflow = workflow_text("secret-scan.yml") From 8800253e0339a4dc434f6b5cb7d789ec2dc69d4e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 22:47:31 +0900 Subject: [PATCH 4/6] refactor(osv): keep base capture external --- .github/workflows/security-scan.yml | 7 ------- CHANGELOG.md | 8 ++++--- .../osv-cross-fork-result-isolation.md | 11 ++++++---- .../test_required_workflow_queue_contract.py | 21 ++++++++++++------- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 59c377b1b..ef8466216 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -134,13 +134,6 @@ jobs: path: source fetch-depth: 0 persist-credentials: false - - name: Restore preserved base OSV output - run: | - set -euo pipefail - src="${RUNNER_TEMP}/osv-old-results.json" - test -s "${src}" - rm -f "${GITHUB_WORKSPACE}/old-results.json" - cp "${src}" "${GITHUB_WORKSPACE}/old-results.json" - name: Discard checkout-provided OSV result files run: | set -euo pipefail diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cbbae699..ad8304f54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,11 @@ Semantic Versioning where the repository publishes a release. - Preserve OSV scan output in private, runner-owned `RUNNER_TEMP` files before a fork head checkout, using a bounded privileged read so root-owned mode-`0600` - scanner output does not depend on world readability; restore by unlinking any - root-owned workspace copy first, discard checkout-provided result files before - each scan, and never treat post-checkout `source/*.json` as reporter input. + scanner output does not depend on world readability; keep the base capture + external during the head scan, materialize reporter inputs exactly once after + unlinking root-owned workspace files, discard checkout-provided result files + before each scan, and never treat post-checkout `source/*.json` as reporter + input. - Honor each trusted base project's exact, integrity-bearing pnpm `packageManager` specification in OpenCode coverage images through the pinned Node distribution's Corepack runtime, instead of admitting the specification diff --git a/docs/doctoring/osv-cross-fork-result-isolation.md b/docs/doctoring/osv-cross-fork-result-isolation.md index 2f33c0920..408b78f95 100644 --- a/docs/doctoring/osv-cross-fork-result-isolation.md +++ b/docs/doctoring/osv-cross-fork-result-isolation.md @@ -31,8 +31,10 @@ GitHub-hosted Linux runner's passwordless `/usr/bin/sudo` solely for a fixed `umask 077`. The resulting `RUNNER_TEMP` capture is therefore runner-owned and mode `0600`; the workflow verifies both ownership and non-empty content. It does not `chmod` the scanner output, grant other users read access, or make the -reporter privileged. Restore by unlinking the workspace file first, then -copying from `RUNNER_TEMP`. Discard `source/old-results.json` and +reporter privileged. Keep the base capture outside the workspace throughout +the head scan; no consumer needs a pre-scan workspace copy. Materialize both +reporter inputs exactly once by unlinking the scanner-created workspace files +and copying from `RUNNER_TEMP`. Discard `source/old-results.json` and `source/new-results.json` before each scan so a fork cannot plant reporter input. After the head checkout, never treat checkout-path JSON as scanner output. Missing or empty captured output remains a hard failure; a zero-finding @@ -42,8 +44,9 @@ vulnerability comparison. ## Verification and rollback - The workflow contract proves both checkouts target `source/`, every scan reads - that same directory, captures land in `RUNNER_TEMP` before compare, restore - unlinks before copy, and post-checkout `source/*.json` is not reporter input. + that same directory, captures land in `RUNNER_TEMP` before compare, reporter + materialization unlinks before copy, and post-checkout `source/*.json` is not + reporter input. - The executable regression runs both production capture steps against an unreadable scanner result through a bounded privilege stand-in, then proves the capture contains the exact result and is runner-owned with mode `0600`. diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index e59349d3c..698c3737c 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1258,7 +1258,7 @@ def test_security_scan_skips_dependency_review_when_dependency_graph_is_unavaila def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> None: - """Limit cross-fork replacement to a child checkout directory.""" + """Keep captures external until the reporter materializes its inputs.""" workflow = workflow_text("security-scan.yml") assert workflow.count("--allow-no-lockfiles") == 4 @@ -1268,7 +1268,7 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non assert workflow.count("-r\n source/") == 4 assert "clean: false" not in workflow assert "Preserve base OSV output outside the checkout path" in workflow - assert "Restore preserved base OSV output" in workflow + assert "Restore preserved base OSV output" not in workflow assert "Capture head OSV output outside the checkout path" in workflow assert "${RUNNER_TEMP}/osv-old-results.json" in workflow assert "${RUNNER_TEMP}/osv-new-results.json" in workflow @@ -1278,24 +1278,29 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non " - name: Preserve base OSV output outside the checkout path" ) checkout_head = workflow.index(" - name: Checkout head") - restore = workflow.index(" - name: Restore preserved base OSV output") + discard_after_checkout = workflow.index( + " - name: Discard checkout-provided OSV result files", checkout_head + ) scan_head = workflow.index(" - name: Scan head with OSV") capture_head = workflow.index( " - name: Capture head OSV output outside the checkout path" ) require_output = workflow.index(" - name: Require OSV scan output") - assert preserve < checkout_head < restore < scan_head < capture_head < require_output - discard_after_restore = workflow.index( - " - name: Discard checkout-provided OSV result files", restore + assert ( + preserve + < checkout_head + < discard_after_checkout + < scan_head + < capture_head + < require_output ) - restore_block = workflow[restore:discard_after_restore] require_block = workflow[ require_output : workflow.index(" - name: Print OSV findings being compared") ] - assert "source/old-results.json" not in restore_block assert "source/old-results.json" not in require_block assert "source/new-results.json" not in require_block assert 'cp "${dest}" "${GITHUB_WORKSPACE}/old-results.json"' not in workflow + assert 'cp "${src}" "${GITHUB_WORKSPACE}/old-results.json"' not in workflow assert "rm -f source/old-results.json source/new-results.json" in workflow assert 'test -s "${old}"' in require_block assert 'test -s "${new}"' in require_block From 7b8fc201ed7d86f415568d0e8df5a9211e1c8ab0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 22:57:36 +0900 Subject: [PATCH 5/6] fix(osv): upload runner-owned debug captures --- .github/workflows/security-scan.yml | 4 ++-- CHANGELOG.md | 4 ++-- docs/doctoring/osv-cross-fork-result-isolation.md | 6 +++++- tests/test_required_workflow_queue_contract.py | 5 +++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index ef8466216..7fbf3f13b 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -292,8 +292,8 @@ jobs: with: name: osv-scan-debug path: | - old-results.json - new-results.json + ${{ runner.temp }}/osv-old-results.json + ${{ runner.temp }}/osv-new-results.json results.sarif if-no-files-found: ignore retention-days: 5 diff --git a/CHANGELOG.md b/CHANGELOG.md index ad8304f54..63e897af3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,8 @@ Semantic Versioning where the repository publishes a release. scanner output does not depend on world readability; keep the base capture external during the head scan, materialize reporter inputs exactly once after unlinking root-owned workspace files, discard checkout-provided result files - before each scan, and never treat post-checkout `source/*.json` as reporter - input. + before each scan, upload runner-owned captures for failure diagnostics, and + never treat post-checkout `source/*.json` as reporter input. - Honor each trusted base project's exact, integrity-bearing pnpm `packageManager` specification in OpenCode coverage images through the pinned Node distribution's Corepack runtime, instead of admitting the specification diff --git a/docs/doctoring/osv-cross-fork-result-isolation.md b/docs/doctoring/osv-cross-fork-result-isolation.md index 408b78f95..b169a39eb 100644 --- a/docs/doctoring/osv-cross-fork-result-isolation.md +++ b/docs/doctoring/osv-cross-fork-result-isolation.md @@ -39,7 +39,9 @@ and copying from `RUNNER_TEMP`. Discard `source/old-results.json` and input. After the head checkout, never treat checkout-path JSON as scanner output. Missing or empty captured output remains a hard failure; a zero-finding head scan does not skip the base comparison. This change does not weaken -vulnerability comparison. +vulnerability comparison. The always-run debug upload reads the private runner +captures directly rather than root-owned workspace results, so an early failure +does not replace the primary diagnostic with an artifact permission error. ## Verification and rollback @@ -50,6 +52,8 @@ vulnerability comparison. - The executable regression runs both production capture steps against an unreadable scanner result through a bounded privilege stand-in, then proves the capture contains the exact result and is runner-owned with mode `0600`. +- The artifact contract uploads the runner-owned captures, including on failure, + and never asks the uploader to read root-owned workspace result files. - `actionlint` validates the edited workflow. - Rerun a fork PR's `Security Scan`; both captured result files must be non-empty before the reporter runs. diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 698c3737c..ea5da886e 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1297,6 +1297,7 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non require_block = workflow[ require_output : workflow.index(" - name: Print OSV findings being compared") ] + debug_upload = workflow_step(workflow, "Upload OSV debug artifacts") assert "source/old-results.json" not in require_block assert "source/new-results.json" not in require_block assert 'cp "${dest}" "${GITHUB_WORKSPACE}/old-results.json"' not in workflow @@ -1304,6 +1305,10 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non assert "rm -f source/old-results.json source/new-results.json" in workflow assert 'test -s "${old}"' in require_block assert 'test -s "${new}"' in require_block + assert "${{ runner.temp }}/osv-old-results.json" in debug_upload + assert "${{ runner.temp }}/osv-new-results.json" in debug_upload + assert "\n old-results.json\n" not in debug_upload + assert "\n new-results.json\n" not in debug_upload @pytest.mark.parametrize( From 20d72bc838d7f91b74ce01bb4de16d07144fa270 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 23:07:17 +0900 Subject: [PATCH 6/6] fix(osv): remove planted result paths robustly --- .github/workflows/security-scan.yml | 4 ++-- CHANGELOG.md | 7 ++++--- .../doctoring/osv-cross-fork-result-isolation.md | 16 +++++++++------- tests/test_required_workflow_queue_contract.py | 12 +++++++++++- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 7fbf3f13b..f16049acc 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -77,7 +77,7 @@ jobs: - name: Discard checkout-provided OSV result files run: | set -euo pipefail - rm -f source/old-results.json source/new-results.json + rm -rf -- source/old-results.json source/new-results.json - name: Scan base with OSV id: osv_base continue-on-error: true @@ -137,7 +137,7 @@ jobs: - name: Discard checkout-provided OSV result files run: | set -euo pipefail - rm -f source/old-results.json source/new-results.json + rm -rf -- source/old-results.json source/new-results.json - name: Scan head with OSV id: osv_head continue-on-error: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 63e897af3..93cd9e92e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,10 @@ Semantic Versioning where the repository publishes a release. fork head checkout, using a bounded privileged read so root-owned mode-`0600` scanner output does not depend on world readability; keep the base capture external during the head scan, materialize reporter inputs exactly once after - unlinking root-owned workspace files, discard checkout-provided result files - before each scan, upload runner-owned captures for failure diagnostics, and - never treat post-checkout `source/*.json` as reporter input. + unlinking root-owned workspace files, discard checkout-provided result files, + links, or directories before each scan, upload runner-owned captures for + failure diagnostics, and never treat post-checkout `source/*.json` as + reporter input. - Honor each trusted base project's exact, integrity-bearing pnpm `packageManager` specification in OpenCode coverage images through the pinned Node distribution's Corepack runtime, instead of admitting the specification diff --git a/docs/doctoring/osv-cross-fork-result-isolation.md b/docs/doctoring/osv-cross-fork-result-isolation.md index b169a39eb..191524825 100644 --- a/docs/doctoring/osv-cross-fork-result-isolation.md +++ b/docs/doctoring/osv-cross-fork-result-isolation.md @@ -35,13 +35,15 @@ reporter privileged. Keep the base capture outside the workspace throughout the head scan; no consumer needs a pre-scan workspace copy. Materialize both reporter inputs exactly once by unlinking the scanner-created workspace files and copying from `RUNNER_TEMP`. Discard `source/old-results.json` and -`source/new-results.json` before each scan so a fork cannot plant reporter -input. After the head checkout, never treat checkout-path JSON as scanner -output. Missing or empty captured output remains a hard failure; a zero-finding -head scan does not skip the base comparison. This change does not weaken -vulnerability comparison. The always-run debug upload reads the private runner -captures directly rather than root-owned workspace results, so an early failure -does not replace the primary diagnostic with an artifact permission error. +`source/new-results.json` as exact paths before each scan, whether a fork plants +a file, link, or directory there, so the planted entry cannot abort the scan or +become reporter input. After the head checkout, never treat checkout-path JSON +as scanner output. Missing or empty captured output remains a hard failure; a +zero-finding head scan does not skip the base comparison. This change does not +weaken vulnerability comparison. The always-run debug upload reads the private +runner captures directly rather than root-owned workspace results, so an early +failure does not replace the primary diagnostic with an artifact permission +error. ## Verification and rollback diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index ea5da886e..9378d7474 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1302,7 +1302,12 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non assert "source/new-results.json" not in require_block assert 'cp "${dest}" "${GITHUB_WORKSPACE}/old-results.json"' not in workflow assert 'cp "${src}" "${GITHUB_WORKSPACE}/old-results.json"' not in workflow - assert "rm -f source/old-results.json source/new-results.json" in workflow + assert ( + workflow.count( + "rm -rf -- source/old-results.json source/new-results.json" + ) + == 2 + ) assert 'test -s "${old}"' in require_block assert 'test -s "${new}"' in require_block assert "${{ runner.temp }}/osv-old-results.json" in debug_upload @@ -1319,6 +1324,11 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non "old-results.json", "osv-old-results.json", ), + ( + "Preserve base OSV output outside the checkout path", + "source/old-results.json", + "osv-old-results.json", + ), ( "Capture head OSV output outside the checkout path", "source/new-results.json",