From d64c7cf12b1d3d783d73c17e9d96196c064c38bc Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Tue, 13 Oct 2026 21:46:03 +0800 Subject: [PATCH] feat: allow immutable Behavior Diff core overrides Signed-off-by: Kent Huang --- README.md | 7 +++++++ SECURITY.md | 5 +++++ action.yml | 4 ++++ docs/configuration.md | 47 ++++++++++++++++++++++++++++++++++++++++--- scripts/common.py | 9 ++++++--- scripts/execute.py | 15 +++++++------- scripts/prepare.py | 5 +++-- scripts/publish.py | 4 ++-- tests/test_execute.py | 15 ++++++++++++-- tests/test_prepare.py | 33 ++++++++++++++++++++++++++++-- tests/test_publish.py | 30 ++++++++++++++++++++++++--- 11 files changed, 150 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index ea717ca..0669a00 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,13 @@ comparisons only on content you have approved for that disclosure. and access the network. Use ephemeral runners, keep unrelated secrets out of the execution job, and read [SECURITY.md](SECURITY.md) before enabling credentials. +For core development and dogfooding, set the optional +[`core-ref` input](docs/configuration.md#testing-a-different-core-revision) in +prepare to a reviewed full commit SHA. Run and publish inherit the frozen +revision. Omit it to keep the release's default core pin. An override runs +different executable code with model credentials; review it separately from +the instruction change being compared. + ## Reading the result `complete` means execution and extraction finished, not that the rule is diff --git a/SECURITY.md b/SECURITY.md index 1d769f1..d1617d1 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -18,6 +18,11 @@ execution. **It is not an untrusted-PR sandbox.** approval of the exact current head SHA by comment or manual dispatch. A new push requires a new approval. Same-repository opt-in is an authorization policy, not proof that the repository is safe. +- A `core-ref` override selects executable code from the core repository. Review + that exact SHA before exposing model credentials; comparison-head approval + does not separately approve the selected core. Protect this input with the + workflow and environment, and do not automatically trust arbitrary PR heads. + SHA validation and checkout verification establish identity, not safety. - Never use `pull_request_target` to execute PR code; the Action rejects it. If adding your own checkout steps, set `persist-credentials: false` and do not check out PR code in preparation/publication jobs. diff --git a/action.yml b/action.yml index 756553c..a0acd14 100644 --- a/action.yml +++ b/action.yml @@ -14,6 +14,9 @@ inputs: selection: description: One frozen matrix entry from prepare, serialized with toJSON(matrix). default: '' + core-ref: + description: Optional reviewed lowercase 40-character Behavior Diff core commit SHA. Prepare freezes it; run/publish inherit it from selection. Empty uses the release pin. + default: '' pr: description: PR number for manual dispatch; event PR identity otherwise. default: '' @@ -116,6 +119,7 @@ runs: BD_MODE: ${{ inputs.mode }} BD_GITHUB_TOKEN: ${{ inputs.github-token }} BD_SELECTION: ${{ inputs.selection }} + BD_CORE_REF: ${{ inputs.core-ref }} BD_PR: ${{ inputs.pr }} BD_APPROVED_SHA: ${{ inputs.approved-sha }} BD_FILE: ${{ inputs.file }} diff --git a/docs/configuration.md b/docs/configuration.md index 85d297d..b624113 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -7,7 +7,7 @@ | Mode | Responsibility | Job permissions | | --- | --- | --- | -| `prepare` | Authorize event, freeze head and merge-base, select targets | `contents: read`, `pull-requests: read` | +| `prepare` | Authorize event, freeze core/head/merge-base revisions, select targets | `contents: read`, `pull-requests: read` | | `run` | Reauthorize, stage one target, draft purpose/task, invoke core, upload selected reports | `contents: read`, `pull-requests: read`; model credential only here | | `publish` | Read the current invocation's artifacts; create/update bot-owned comments | `actions: read`, `pull-requests: write`, `contents: read`; no model credential | @@ -31,6 +31,7 @@ publisher also suppresses stale heads and older run/attempt results. | `mode` | required | `prepare`, `run`, or `publish` | | `github-token` | `github.token` | Read-only during preparation/execution; workflow token for publication | | `selection` | empty | In run/publish, `toJSON(matrix)` from prepare's frozen `matrix`; never PR-supplied JSON | +| `core-ref` | release core pin | Optional reviewed lowercase 40-character commit SHA from `spacedock-dev/behavior-diff`; set in prepare, inherited through selection in run/publish | | `pr` | event identity | Manual dispatch: canonical decimal number (no leading zeros); cannot override a different event PR | | `approved-sha` | empty | Manual dispatch: current lowercase 40-character head SHA; comment approval comes from the command | | `file` | all eligible | Exact changed instruction path; quote spaces in comment commands | @@ -49,7 +50,47 @@ publisher also suppresses stale heads and older run/attempt results. Pass the same request/trial/trust inputs to prepare and run: execution checks the request again. A moved head or base invalidates the frozen selection before paid calls. Each matrix entry is bound to the repository, PR, head/base, -merge-base, target, trial count, workflow run ID and attempt. +merge-base, core revision, target, trial count, workflow run ID and attempt. + +### Testing a different core revision + +Set `core-ref` on the **prepare** step to use a reviewed core commit instead of +the Action release's default pin. For example: + +```yaml +with: + mode: prepare + core-ref: a5dacb04b26b6562cff6759004251888f7250ef5 +``` + +Keep the rest of the caller workflow unchanged. Prepare freezes the revision +in each matrix entry as `core_revision`. Run and publish inherit it from +`selection`; do not resolve a branch again in later jobs. If you also supply +`core-ref` in run or publish, it must match the prepared revision. + +Only a full lowercase 40-character commit SHA is accepted. Branches, tags, +abbreviations and repository overrides are rejected. Run fetches only from +`spacedock-dev/behavior-diff`, checks out that exact commit and verifies `HEAD`. +An unavailable commit fails execution; there is no fallback to the release pin. +The result record retains the selection, and the comment names the full selected +core SHA even when no report was produced. Artifact evidence from a different +core revision is rejected. + +Use the same Action version in all three jobs. Selection schema 2 includes +the core revision; older selections are rejected rather than assigned a pin. +Leaving `core-ref` empty in prepare keeps the default below. + +**A core override selects executable code, not just instruction text.** Review +the exact commit before allowing it access to model credentials. A same-repository +PR or an approval of the comparison's head does not by itself approve a different +core revision. Protect the caller workflow and execution environment; do not +automatically use arbitrary PR-head code with secrets. + +The override does not change environment isolation. The runner still receives +no GitHub environment variables or event payload, so selecting a core with CI +header detection does not enable those headers through this Action. That needs +a separate integration carrying only validated report metadata, not the full +CI environment. ### Outputs and status @@ -109,7 +150,7 @@ requirement; Ubuntu supplies Bash, Python 3, Git and jq. | Dependency | Pin | | --- | --- | -| Behavior Diff core | v0.4.6, `5b325ed40062bb819c574e4172da04f53b53b953` | +| Behavior Diff core (default) | v0.4.6, `5b325ed40062bb819c574e4172da04f53b53b953` | | Claude Code | `@anthropic-ai/claude-code@2.1.296` | | upload-artifact | v7.0.2, `cf430e030ddbb5b0abf93d22962f4752f3646cd9` | | setup-node | v4.4.0, immutable SHA in `action.yml` | diff --git a/scripts/common.py b/scripts/common.py index d11f10d..2ee4632 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -59,12 +59,15 @@ def target_id(path): def validate_selection(value): fields = {"schema", "repository", "pr", "head", "base", "merge_base", "path", - "previous_path", "change", "trials", "id", "run_id", "run_attempt"} - if not isinstance(value, dict) or set(value) != fields or value["schema"] != 1: + "previous_path", "change", "trials", "id", "run_id", "run_attempt", "core_revision"} + if not isinstance(value, dict) or set(value) != fields or type(value["schema"]) is not int or value["schema"] != 2: raise ValueError("Invalid selection schema") repository_name(value["repository"]) - for key in ("head", "base", "merge_base"): + for key in ("head", "base", "merge_base", "core_revision"): sha(value[key]) + requested_core = input_value("core-ref") + if requested_core and sha(requested_core) != value["core_revision"]: + raise ValueError("core-ref does not match the prepared selection") for key in ("path", "previous_path"): safe_path(value[key]) if PurePosixPath(value["path"]).name not in {"SKILL.md", "AGENTS.md", "CLAUDE.md"}: diff --git a/scripts/execute.py b/scripts/execute.py index 05eaf8e..cf70888 100644 --- a/scripts/execute.py +++ b/scripts/execute.py @@ -28,7 +28,7 @@ import time from typing import NamedTuple -from common import (CLAUDE_VERSION, CORE_REVISION, clean_env, fetch_repository, git, +from common import (CLAUDE_VERSION, clean_env, fetch_repository, git, input_value, integer, private_directory, run_command, safe_path, sha, validate_selection, write_private) from prepare import changed_files, regular_blob, resolve_request @@ -436,8 +436,9 @@ def _remaining(deadline, cap): return min(cap, left) -def install_dependencies(work, deadline): - """Install pinned Claude Code into a private prefix and clone the pinned core.""" +def install_dependencies(work, deadline, core_revision): + """Install pinned Claude Code and the exact core frozen during preparation.""" + sha(core_revision) prefix = work / "deps" / "claude" prefix.mkdir(parents=True, mode=0o700) (work / "deps" / "home").mkdir(mode=0o700) @@ -468,9 +469,9 @@ def install_dependencies(work, deadline): core = work / "deps" / "core" core.mkdir(mode=0o700) _git(["init", "-q"], core, deadline) - _git(["fetch", "-q", "--depth", "1", "--no-tags", "--", CORE_URL, CORE_REVISION], core, deadline, 600) - _git(["-c", "advice.detachedHead=false", "checkout", "-q", "--detach", CORE_REVISION], core, deadline) - if _git(["rev-parse", "HEAD"], core, deadline).decode().strip() != CORE_REVISION: + _git(["fetch", "-q", "--depth", "1", "--no-tags", "--", CORE_URL, core_revision], core, deadline, 600) + _git(["-c", "advice.detachedHead=false", "checkout", "-q", "--detach", core_revision], core, deadline) + if _git(["rev-parse", "HEAD"], core, deadline).decode().strip() != core_revision: raise Failure("dependencies") return Dependencies(claude, core / CORE_SCRIPTS) @@ -801,7 +802,7 @@ def _compare(selection, api, options, work, deadline, state, token): paths = project_paths(project, deadline) state["phase"] = "dependencies" - dependencies = install_dependencies(work, deadline) + dependencies = install_dependencies(work, deadline, selection["core_revision"]) # Drafts get their own HOME, removed before trials, so trial sessions never load draft state. draft_env = model_env(work, dependencies, token, work / "draft-home") env = model_env(work, dependencies, token, work / "home") diff --git a/scripts/prepare.py b/scripts/prepare.py index ecc9417..ada2b88 100644 --- a/scripts/prepare.py +++ b/scripts/prepare.py @@ -6,7 +6,7 @@ import shutil import urllib.parse -from common import (fetch_repository, git, input_value, integer, private_directory, +from common import (CORE_REVISION, fetch_repository, git, input_value, integer, private_directory, repository_name, safe_path, sha, target_id, validate_selection) @@ -151,6 +151,7 @@ def regular_blob(directory, commit, path, deadline=None): def prepare(api): + core_revision = sha(input_value("core-ref") or CORE_REVISION) pr, requested, trials = resolve_request(api) repository = os.environ["GITHUB_REPOSITORY"] head, base = pr["head"]["sha"], pr["base"]["sha"] @@ -172,7 +173,7 @@ def prepare(api): regular_blob(repo, head, entry["path"]) if entry["change"] != "A": regular_blob(repo, merge_base, entry["previous_path"]) - selection = dict(schema=1, repository=repository, pr=pr["number"], head=head, + selection = dict(schema=2, core_revision=core_revision, repository=repository, pr=pr["number"], head=head, base=base, merge_base=merge_base, trials=trials, id=target_id(entry["path"]), run_id=os.environ["GITHUB_RUN_ID"], run_attempt=os.environ["GITHUB_RUN_ATTEMPT"], **entry) selections.append(validate_selection(selection)) diff --git a/scripts/publish.py b/scripts/publish.py index d6af97b..5f7d235 100644 --- a/scripts/publish.py +++ b/scripts/publish.py @@ -26,7 +26,7 @@ import zipfile import zlib -from common import CORE_REVISION, NoRedirect, input_value, validate_selection +from common import NoRedirect, input_value, validate_selection SERVER = "https://github.com" API = "https://api.github.com" @@ -493,7 +493,7 @@ def _comment(selection, evidence, run_url): "", "---", f"Generated deterministically from this run's result record and Behavior Diff core report " - f"(core {_code(CORE_REVISION[:12])}); no model wrote this comment. Observed trial behavior is " + f"(selected core {_code(selection['core_revision'])}); no model wrote this comment. Observed trial behavior is " "evidence to review, not a pass/fail verdict.", ] body = "\n".join(line for index, line in enumerate(lines) diff --git a/tests/test_execute.py b/tests/test_execute.py index 81d6671..466b324 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -181,6 +181,7 @@ def setUp(self): "GITHUB_TOKEN": GITHUB_SECRET, "GH_TOKEN": GITHUB_SECRET, "BD_GITHUB_TOKEN": GITHUB_SECRET, + "BD_CORE_REF": "", "INPUT_GITHUB-TOKEN": GITHUB_SECRET, "ACTIONS_RUNTIME_TOKEN": GITHUB_SECRET, "CLAUDE_CODE_OAUTH_TOKEN": MODEL_SECRET, @@ -248,7 +249,8 @@ def source(self, base, head, base_links=None): return merge_base, run_git(source, "rev-parse", "HEAD") def selection(self, merge_base, head, path, change, previous=None, trials=3): - return {"schema": 1, "repository": REPOSITORY, "pr": 7, "head": head, "base": merge_base, + return {"schema": 2, "core_revision": common.CORE_REVISION, + "repository": REPOSITORY, "pr": 7, "head": head, "base": merge_base, "merge_base": merge_base, "path": path, "previous_path": previous or path, "change": change, "trials": trials, "id": common.target_id(path), "run_id": "101", "run_attempt": "1"} @@ -268,7 +270,7 @@ def run_execute(self, selection, options=None, pr=None, trials=None): with mock.patch.object(execute, "resolve_request", return_value=authorized), \ mock.patch.object(execute, "fetch_repository", side_effect=self.fetch), \ mock.patch.object(execute, "install_dependencies", - side_effect=lambda work, deadline: self.dependencies): + side_effect=lambda work, deadline, core_revision: self.dependencies): return execute.execute(selection, types.SimpleNamespace(token=GITHUB_SECRET), {} if options is None else options) @@ -300,6 +302,15 @@ def assert_failed_before_trials(self, outputs, selection, error_key, model_calls self.assertEqual(self.records(), []) self.assertFalse((Path(outputs["scratch-directory"]) / "work").exists()) + def test_changed_core_ref_fails_before_execution(self): + selection = self.selection("a" * 40, "b" * 40, "CLAUDE.md", "M") + with mock.patch.dict(os.environ, {"BD_CORE_REF": "d" * 40}): + with self.assertRaisesRegex(ValueError, "core-ref does not match"): + self.run_execute(selection) + self.assertEqual(self.calls(), []) + self.assertEqual(self.records(), []) + self.assertEqual(list(self.runner_temp.iterdir()), []) + # ------------------------------------------------------------ staging and isolation def test_modified_instruction_runs_isolated_and_complete(self): diff --git a/tests/test_prepare.py b/tests/test_prepare.py index 0f9288e..66d45de 100644 --- a/tests/test_prepare.py +++ b/tests/test_prepare.py @@ -11,7 +11,7 @@ from unittest.mock import patch sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts")) -from common import clean_env, git, safe_path, target_id, validate_selection +from common import CORE_REVISION, clean_env, git, safe_path, target_id, validate_selection from prepare import authorize, changed_files, parse_command, prepare, regular_blob @@ -40,7 +40,7 @@ def setUp(self): "GITHUB_ACTOR": "maintainer", "GITHUB_TRIGGERING_ACTOR": "maintainer", "GITHUB_EVENT_NAME": "workflow_dispatch", "BD_PR": "7", "BD_TRIALS": "3", "BD_FAST": "false", "BD_TRUST_POLICY": "explicit", "BD_PUBLICATION": "none", - "BD_FILE": "", "BD_MAX_FILES": "3", "RUNNER_TEMP": str(self.root), + "BD_FILE": "", "BD_MAX_FILES": "3", "BD_CORE_REF": "", "RUNNER_TEMP": str(self.root), "GITHUB_WORKSPACE": str(self.repo), "BD_GITHUB_TOKEN": "synthetic-token", }) self.env.start() @@ -160,11 +160,40 @@ def test_prepare_freezes_matrix_and_cleans_scratch(self): selection = result["matrix"]["include"][0] self.assertEqual((result["status"], result["count"], selection["merge_base"]), ("ready", 1, self.base)) self.assertEqual(selection["path"], "CLAUDE.md") + self.assertEqual(selection["core_revision"], CORE_REVISION) self.assertEqual(list(self.root.glob("behavior-diff-prepare-*")), []) with patch.dict(os.environ, {"GITHUB_RUN_ATTEMPT": "2"}): with self.assertRaises(ValueError): validate_selection(selection) + def test_core_ref_is_frozen_and_inherited_without_repeating_input(self): + revision = "d" * 40 + with patch("prepare.fetch_repository", self.fetch), patch.dict(os.environ, {"BD_CORE_REF": revision}): + selection = prepare(API(self.pr))["matrix"]["include"][0] + self.assertEqual(validate_selection(selection)["core_revision"], revision) + with patch.dict(os.environ, {"BD_CORE_REF": CORE_REVISION}): + with self.assertRaisesRegex(ValueError, "core-ref does not match"): + validate_selection(selection) + + def test_core_ref_rejects_mutable_and_malformed_refs_before_fetch(self): + for revision in ("main", "v0.4.6", "abcdef0", "A" * 40, "a" * 64, + "a" * 40 + "\n", " " + "a" * 40, "--upload-pack=bad"): + with self.subTest(revision=revision), patch.dict(os.environ, {"BD_CORE_REF": revision}): + with patch("prepare.fetch_repository") as fetch, self.assertRaises(ValueError): + prepare(API(self.pr)) + fetch.assert_not_called() + + def test_selection_requires_valid_frozen_core(self): + with patch("prepare.fetch_repository", self.fetch): + selection = prepare(API(self.pr))["matrix"]["include"][0] + for revision in (None, "", "main", 123, "d" * 39): + with self.subTest(revision=revision), self.assertRaises(ValueError): + validate_selection({**selection, "core_revision": revision}) + legacy = {key: value for key, value in selection.items() if key != "core_revision"} + legacy["schema"] = 1 + with self.assertRaisesRegex(ValueError, "selection schema"): + validate_selection(legacy) + def test_deletion_only_auto_skips_but_explicit_fails(self): (self.repo / "CLAUDE.md").unlink() head = self.commit() diff --git a/tests/test_publish.py b/tests/test_publish.py index c3eaf4c..aabb732 100644 --- a/tests/test_publish.py +++ b/tests/test_publish.py @@ -14,7 +14,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts")) -from common import target_id # noqa: E402 +from common import CORE_REVISION, target_id # noqa: E402 import publish # noqa: E402 HEAD, BASE, MERGE_BASE = "a" * 40, "b" * 40, "c" * 40 @@ -69,7 +69,8 @@ def make_selection(path=PATH, change="M", previous_path=None): - return {"schema": 1, "repository": "octo/repo", "pr": 7, "head": HEAD, "base": BASE, + return {"schema": 2, "core_revision": CORE_REVISION, + "repository": "octo/repo", "pr": 7, "head": HEAD, "base": BASE, "merge_base": MERGE_BASE, "path": path, "previous_path": previous_path or path, "change": change, "trials": 3, "id": target_id(path), "run_id": "123", "run_attempt": "2"} @@ -202,7 +203,8 @@ def marker_comment(comment_id, run, attempt, target, login=BOT, kind="Bot", pref class PublicationTest(unittest.TestCase): def setUp(self): environment = mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": "octo/repo", "GITHUB_RUN_ID": "123", - "GITHUB_RUN_ATTEMPT": "2", "BD_PUBLICATION": "comment"}) + "GITHUB_RUN_ATTEMPT": "2", "BD_PUBLICATION": "comment", + "BD_CORE_REF": ""}) environment.start() self.addCleanup(environment.stop) self.selection = make_selection() @@ -262,6 +264,28 @@ def test_neutralizes_untrusted_markup(self): class PublishTests(PublicationTest): + def test_custom_core_provenance_comes_from_frozen_selection(self): + self.selection["core_revision"] = "d" * 40 + api = FakeAPI(self.selection) + outputs, _ = self.publish(api) + self.assertEqual(outputs["report-status"], "complete") + self.assertIn("`" + "d" * 40 + "`", api.body()) + self.assertNotIn(CORE_REVISION, api.body()) + + def test_result_from_different_core_is_not_published_as_evidence(self): + self.selection["core_revision"] = "d" * 40 + api = FakeAPI(self.selection) + record = result({**self.selection, "core_revision": CORE_REVISION}) + outputs, _ = self.publish(api, report_archive(record)) + self.assert_failed(api, outputs, publish.FOREIGN_RESULT) + + def test_conflicting_core_input_fails_before_publication(self): + api = FakeAPI(self.selection) + with mock.patch.dict(os.environ, {"BD_CORE_REF": "d" * 40}): + with self.assertRaisesRegex(ValueError, "core-ref does not match"): + self.publish(api) + self.assertEqual(api.calls, []) + def test_skips_without_comment_policy(self): for policy in ("none", "artifact"): with self.subTest(policy), mock.patch.dict(os.environ, {"BD_PUBLICATION": policy}):