Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 51 additions & 4 deletions .github/workflows/security-scan.yml
Comment thread
seonghobae marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -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 -rf -- source/old-results.json source/new-results.json
- name: Scan base with OSV
id: osv_base
continue-on-error: true
Expand Down Expand Up @@ -105,6 +109,23 @@ jobs:
--allow-no-lockfiles
-r
source/
Comment thread
seonghobae marked this conversation as resolved.
- name: Preserve base OSV output outside the checkout path
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 [ -f "${candidate}" ] && [ ! -L "${candidate}" ] && [ -s "${candidate}" ]; then
/usr/bin/sudo --non-interactive /usr/bin/cat -- "${candidate}" | /usr/bin/tee "${dest}" >/dev/null
Comment thread
seonghobae marked this conversation as resolved.
test -s "${dest}"
test -O "${dest}"
echo "Preserved OSV base output from ${candidate}"
exit 0
fi
done
echo "::error::OSV base output was not present after the base scan steps."
exit 1
Comment thread
seonghobae marked this conversation as resolved.
- name: Checkout head
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
Expand All @@ -113,6 +134,10 @@ jobs:
path: source
fetch-depth: 0
persist-credentials: false
- name: Discard checkout-provided OSV result files
run: |
set -euo pipefail
rm -rf -- source/old-results.json source/new-results.json
- name: Scan head with OSV
id: osv_head
continue-on-error: true
Expand Down Expand Up @@ -144,11 +169,33 @@ 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"
rm -f "${dest}"
umask 077
for candidate in new-results.json source/new-results.json; do
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
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
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
Comment thread
seonghobae marked this conversation as resolved.
Comment thread
seonghobae marked this conversation as resolved.
- name: Print OSV findings being compared
shell: python3 {0}
run: |
Expand Down Expand Up @@ -245,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
Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ Semantic Versioning where the repository publishes a release.

## [Unreleased]

- 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; 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,
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
Expand Down
55 changes: 46 additions & 9 deletions docs/doctoring/osv-cross-fork-result-isolation.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,57 @@ 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.
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

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. 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). 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. 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` 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

- 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, 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`.
- 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 `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.

Expand All @@ -34,6 +67,10 @@ a hard failure; this change does not 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

Expand Down
134 changes: 130 additions & 4 deletions tests/test_required_workflow_queue_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import shlex
import shutil
import stat
import subprocess
import sys
import textwrap
Expand Down Expand Up @@ -1257,17 +1258,142 @@ 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
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 "test -s old-results.json" in workflow
assert "test -s new-results.json" in workflow
assert "Preserve base OSV output outside the checkout path" 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
assert "source/old-results.json" in workflow
Comment thread
seonghobae marked this conversation as resolved.
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")
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
< discard_after_checkout
< scan_head
< capture_head
< require_output
)
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
assert 'cp "${src}" "${GITHUB_WORKSPACE}/old-results.json"' not 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
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(
("step_name", "candidate_name", "destination_name"),
[
(
"Preserve base OSV output outside the checkout path",
"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",
"osv-new-results.json",
),
],
Comment thread
seonghobae marked this conversation as resolved.
)
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:
Expand Down
Loading