Skip to content
Open
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 @@ -156,6 +156,10 @@ jobs:
exit 1
fi
echo "SECURITY_CHECKOUT scanner=osv revision=base repository=${EXPECTED_CHECKOUT_REPOSITORY} expected_sha=${EXPECTED_CHECKOUT_SHA} actual_sha=${actual_sha}"
- 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 @@ -187,6 +191,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}"
Comment on lines +199 to +205

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Root-owned output remains consumable

Privileged cat reads the scanner result, while unprivileged tee creates the private capture. Later steps therefore retain readable, runner-owned evidence.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

exit 0
fi
done
Comment on lines +197 to +208

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Failed scans cannot reuse stale captures

rm -f clears each temporary capture before validation. Scan-root cleanup also prevents a failed scan from reusing checkout-provided results.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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 @@ -207,6 +228,10 @@ jobs:
exit 1
fi
echo "SECURITY_CHECKOUT scanner=osv revision=head repository=${EXPECTED_CHECKOUT_REPOSITORY} expected_sha=${EXPECTED_CHECKOUT_SHA} actual_sha=${actual_sha}"
- 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 @@ -238,11 +263,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.
Comment on lines +286 to +292

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Reporter inputs avoid partial materialization

Both captures pass validation before workspace inputs are replaced. A missing result preserves external evidence and prevents a partial comparison.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

- name: Print OSV findings being compared
shell: python3 {0}
run: |
Expand Down Expand Up @@ -339,8 +386,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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@

- Added `ContextualWisdomLab/governance-risk-compliance` to the `OPENCODE_REPOSITORY_DISPATCH_TARGETS` repository variable directly (the actual source of truth for `ALLOWED_TARGET_REPOSITORIES` in both scheduler workflows) and removed the temporary hardcoded-literal bridge a prior commit had added to `pr-review-merge-scheduler.yml`/`pr-review-fix-scheduler.yml` to work around the variable not yet including it. Hardcoding a specific product repository into these shared scheduler workflows violates this repo's own thin-caller convention (`CLAUDE.md`: "Product hourly callers stay thin. Do not hard-code OriginWeave, aFIPC, naruon, or Keyverse into `pr-review-fix-scheduler.yml`") and broke `test_no_target_repository_is_hard_coded_in_the_shared_scheduler`. Updating the variable achieves the same admission with no code change and no test regression.

### Fork-checkout OSV evidence preservation

- 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.
### Hourly review-repair queue-scan bound

- Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up.
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
135 changes: 131 additions & 4 deletions tests/test_required_workflow_queue_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import json
import os
import re
import shlex
import shutil
import stat
import subprocess
import sys
import textwrap
Expand Down Expand Up @@ -1188,17 +1190,142 @@ def test_dependency_review_transport_failure_cannot_hide_behind_http_200(


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
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