From 138c9f99785a1aa7c41ee9f580bd965ce58bfa49 Mon Sep 17 00:00:00 2001 From: kikashy Date: Sun, 1 Nov 2026 08:31:41 -0500 Subject: [PATCH 1/2] Bind PR reuse to the pinned API merge event Signed-off-by: kikashy --- docs/ci-evidence.md | 36 ++- lib/release_evidence.py | 28 +- scripts/dev/release_evidence.py | 28 +- tests/fixture/github-api-2026-03-10.json | 322 +++++++++++++++++++++++ tests/release_evidence_test.py | 77 +++++- tests/run.sh | 4 +- 6 files changed, 473 insertions(+), 22 deletions(-) create mode 100644 tests/fixture/github-api-2026-03-10.json diff --git a/docs/ci-evidence.md b/docs/ci-evidence.md index ce8f22e..ffb9e55 100644 --- a/docs/ci-evidence.md +++ b/docs/ci-evidence.md @@ -497,7 +497,7 @@ and each of these must hold, or nothing is reused: | Input | Bound by | | --- | --- | -| the trusted relationship | records that survive the merge and that the pull request's code cannot write, read through the API: one pull request of this repository (not a fork, base repository this one) merged into the trusted branch with M as its merge commit; its head branch's listing (`pulls?head=owner:branch&state=all`, every page to an empty one, ten at most) shows exactly that one pull request ever, from this repository; its timeline (every page) has no base-change event of any kind (`base_ref_changed`, `automatic_base_change_succeeded`, `automatic_base_change_failed`, and any event whose name contains `base_change` or `base_ref`); the newest `pull_request` run of the same workflow is at its final head, from this repository, on its head branch, and was created after the pull request was opened. The run's own `pull_requests`, which GitHub clears once a pull request merges, must name exactly that pull request into the trusted branch when it is not empty. Anything of this that cannot be read refuses (`reuse-pull-request`) | +| the trusted relationship | records that survive the merge and that the pull request's code cannot write, read through the API: one pull request of this repository (not a fork, base repository this one) merged into the trusted branch with exactly one timeline `merged` event whose `commit_id` equals M (the pinned API has no `merge_commit_sha` on PR objects); its head branch's listing (`pulls?head=owner:branch&state=all`, every page to an empty one, ten at most) shows exactly that one pull request ever, from this repository; its timeline (every page) has no base-change event of any kind (`base_ref_changed`, `automatic_base_change_succeeded`, `automatic_base_change_failed`, and any event whose name contains `base_change` or `base_ref`); the newest `pull_request` run of the same workflow is at its final head, from this repository, on its head branch, and was created after the pull request was opened. The run's own `pull_requests`, which GitHub clears once a pull request merges, must name exactly that pull request into the trusted branch when it is not empty. Anything of this that cannot be read refuses (`reuse-pull-request`) | | exact source and tree | the merge commit the pull request job checked out (S), read through the API: its parents are M's first parent and the pull request's head, and its tree is M's tree | | the base relationship | S was made on M's first parent: a change set tested on another base is not M's, even with the same tree (a rebase merge of several commits therefore never qualifies) | | workflow, configuration and dependency digests | the pull request job's input identities (the manifest, the configuration, the gate, the workflow files, every pin file) equal M's | @@ -698,3 +698,37 @@ the fresh route's full CI run and a re-run. GitHub's API contracts: [workflow runs](https://docs.github.com/en/rest/actions/workflow-runs), [attempt jobs](https://docs.github.com/en/rest/actions/workflow-jobs), [artifacts](https://docs.github.com/en/rest/actions/artifacts). + + +## Pinned API response audit + +The gate sends API version `2026-03-10`. Read-only response projections captured on +2026-11-01 are in `tests/fixture/github-api-2026-03-10.json`, from +`judgment-pack-agent` PR #267 and run 41149914554, and the annotated +`judgment-pack-devkit` v0.13.0 tag. The fixture records its source identities and the +consumed field paths. Unrelated fields and personal identities are omitted. These +are API contract samples, not execution evidence or generated acceptance vectors. +Handwritten decision/refusal vectors remain in `tests/release_evidence_test.py`. + +| API record | Fields read by the gate | +| --- | --- | +| Repository | `id`, `full_name`, `default_branch` | +| Annotated tag reference/object | reference `ref`; `object.type`, `object.sha` at each peel | +| Workflow | `id`, `path`, `name`, `state` | +| Run listing/single run | `id`, `run_attempt`, `event`, `head_branch`, `head_sha`, `workflow_id`, `path`, `status`, `conclusion`, `created_at`, `run_started_at`, `updated_at`, `repository.id/full_name`, `head_repository.id/full_name`, `pull_requests` | +| Nonempty run PR association | `number`, `base.ref`, `base.repo.id` | +| Job | `id`, `name`, `run_id`, `run_attempt`, `head_sha`, `head_branch`, `workflow_name`, `status`, `conclusion`, `labels`, `runner_group_id`, `runner_group_name`, `started_at`, `completed_at` | +| Artifact | `id`, `name`, `expired`, `size_in_bytes`, `created_at`, `digest`, `workflow_run.id/head_sha/head_branch/repository_id/head_repository_id`; ZIP bytes | +| Git commit | `sha`, `tree.sha`, `parents[].sha` | +| Commit-associated PR | `number`, `state`, `created_at`, `merged_at`, `head.sha/ref/repo.id`, `base.ref/repo.id` | +| Head-branch PR listing | `number`, `head.repo.id` | +| PR timeline | `event`; a `merged` event's `commit_id` | +| Paged wrappers | `total_count` and `workflow_runs`, `jobs` or `artifacts`; every row's `id` for uniqueness | + +All listed paths were checked against the captured records. The merged run has an +empty PR association; a separate captured run, 41223435315, checks the nonempty +association fields. +`merge_commit_sha` is absent on both the commit-associated PR and single-PR +responses. Missing, unreadable, malformed or ambiguous merge events refuse with +`reuse-pull-request`; no PR-supplied record substitutes for the timeline. The same +complete timeline is used for the existing base-change checks. diff --git a/lib/release_evidence.py b/lib/release_evidence.py index 34f7b54..f6b2514 100644 --- a/lib/release_evidence.py +++ b/lib/release_evidence.py @@ -1041,16 +1041,35 @@ def pull_request_of(api, config, source): need(commit['sha'] == sha and commit['parents'], 'reuse-pull-request', 'commit %s' % sha) pulls = api.get('commits/%s/pulls?per_page=100' % sha) need(isinstance(pulls, list) and len(pulls) < 100, 'reuse-pull-request', 'the pull request listing') - merged = [pull for pull in pulls if pull['merge_commit_sha'] == sha and pull['merged_at']] + # The pinned API does not expose merge_commit_sha on pull request objects. The + # server's timeline is the surviving merge-to-commit binding, not PR-authored evidence. + merged = [] + try: + for pull in pulls: + if not pull['merged_at']: + continue + need(positive(pull['number']), 'reuse-pull-request', 'invalid pull request number') + events = listing(api, 'issues/%d/timeline' % pull['number']) + merges = [event for event in events if event.get('event') == 'merged'] + need(len(merges) == 1 and isinstance(merges[0].get('commit_id'), str) and + SHA.fullmatch(merges[0]['commit_id']), 'reuse-pull-request', + 'pull request %d has no unique readable merged event' % pull['number']) + if merges[0]['commit_id'] == sha: + merged.append((pull, events)) + except (Refusal,) + MALFORMED as error: + if isinstance(error, Refusal) and error.code == 'reuse-pull-request': + raise + raise Refusal('reuse-pull-request', 'the merged pull request timeline could not be read: %s' % ( + error.code if isinstance(error, Refusal) else type(error).__name__)) from error need(len(merged) == 1, 'reuse-pull-request', '%d pull requests were merged as %s' % (len(merged), sha)) - pull = merged[0] + pull, events = merged[0] need(pull['state'] == 'closed' and pull['base']['ref'] == config['branch'] and pull['base']['repo']['id'] == source['repository_id'] and positive(pull['number']) and SHA.fullmatch(pull['head']['sha']), 'reuse-pull-request', 'pull request %s is not merged into %s here' % (pull.get('number'), config['branch'])) need((pull['head']['repo'] or {}).get('id') == source['repository_id'], 'reuse-pull-request', 'pull request %s comes from another repository' % pull['number']) - bind_head_branch(api, config, source, pull) + bind_head_branch(api, config, source, pull, events) return pull, commit @@ -1073,7 +1092,7 @@ def listing(api, suffix): BASE_EVENT = re.compile(r'base_change|base_ref') -def bind_head_branch(api, config, source, pull): +def bind_head_branch(api, config, source, pull, events): """Records that survive the merge and that the pull request's code cannot write, binding its head branch to it alone: the head branch has had exactly one pull request ever, the merged one, from this repository (a run of another one executes another base's workflow at the same head), @@ -1082,7 +1101,6 @@ def bind_head_branch(api, config, source, pull): try: pulls = listing(api, 'pulls?head=%s:%s&state=all' % (owner, quote(pull['head']['ref'], safe=''))) listed = [(other['number'], (other['head']['repo'] or {}).get('id')) for other in pulls] - events = listing(api, 'issues/%d/timeline' % pull['number']) names = [event.get('event') for event in events] need(all(isinstance(name, str) or name is None for name in names), 'unavailable-or-malformed') changed = sorted({name for name in names if name and BASE_EVENT.search(name)}) diff --git a/scripts/dev/release_evidence.py b/scripts/dev/release_evidence.py index bd8a2c8..7c84904 100644 --- a/scripts/dev/release_evidence.py +++ b/scripts/dev/release_evidence.py @@ -1042,16 +1042,35 @@ def pull_request_of(api, config, source): need(commit['sha'] == sha and commit['parents'], 'reuse-pull-request', 'commit %s' % sha) pulls = api.get('commits/%s/pulls?per_page=100' % sha) need(isinstance(pulls, list) and len(pulls) < 100, 'reuse-pull-request', 'the pull request listing') - merged = [pull for pull in pulls if pull['merge_commit_sha'] == sha and pull['merged_at']] + # The pinned API does not expose merge_commit_sha on pull request objects. The + # server's timeline is the surviving merge-to-commit binding, not PR-authored evidence. + merged = [] + try: + for pull in pulls: + if not pull['merged_at']: + continue + need(positive(pull['number']), 'reuse-pull-request', 'invalid pull request number') + events = listing(api, 'issues/%d/timeline' % pull['number']) + merges = [event for event in events if event.get('event') == 'merged'] + need(len(merges) == 1 and isinstance(merges[0].get('commit_id'), str) and + SHA.fullmatch(merges[0]['commit_id']), 'reuse-pull-request', + 'pull request %d has no unique readable merged event' % pull['number']) + if merges[0]['commit_id'] == sha: + merged.append((pull, events)) + except (Refusal,) + MALFORMED as error: + if isinstance(error, Refusal) and error.code == 'reuse-pull-request': + raise + raise Refusal('reuse-pull-request', 'the merged pull request timeline could not be read: %s' % ( + error.code if isinstance(error, Refusal) else type(error).__name__)) from error need(len(merged) == 1, 'reuse-pull-request', '%d pull requests were merged as %s' % (len(merged), sha)) - pull = merged[0] + pull, events = merged[0] need(pull['state'] == 'closed' and pull['base']['ref'] == config['branch'] and pull['base']['repo']['id'] == source['repository_id'] and positive(pull['number']) and SHA.fullmatch(pull['head']['sha']), 'reuse-pull-request', 'pull request %s is not merged into %s here' % (pull.get('number'), config['branch'])) need((pull['head']['repo'] or {}).get('id') == source['repository_id'], 'reuse-pull-request', 'pull request %s comes from another repository' % pull['number']) - bind_head_branch(api, config, source, pull) + bind_head_branch(api, config, source, pull, events) return pull, commit @@ -1074,7 +1093,7 @@ def listing(api, suffix): BASE_EVENT = re.compile(r'base_change|base_ref') -def bind_head_branch(api, config, source, pull): +def bind_head_branch(api, config, source, pull, events): """Records that survive the merge and that the pull request's code cannot write, binding its head branch to it alone: the head branch has had exactly one pull request ever, the merged one, from this repository (a run of another one executes another base's workflow at the same head), @@ -1083,7 +1102,6 @@ def bind_head_branch(api, config, source, pull): try: pulls = listing(api, 'pulls?head=%s:%s&state=all' % (owner, quote(pull['head']['ref'], safe=''))) listed = [(other['number'], (other['head']['repo'] or {}).get('id')) for other in pulls] - events = listing(api, 'issues/%d/timeline' % pull['number']) names = [event.get('event') for event in events] need(all(isinstance(name, str) or name is None for name in names), 'unavailable-or-malformed') changed = sorted({name for name in names if name and BASE_EVENT.search(name)}) diff --git a/tests/fixture/github-api-2026-03-10.json b/tests/fixture/github-api-2026-03-10.json new file mode 100644 index 0000000..4eefd9d --- /dev/null +++ b/tests/fixture/github-api-2026-03-10.json @@ -0,0 +1,322 @@ +{ + "api_version": "2026-03-10", + "captured_on": "2026-11-01", + "repository": "Judgment-Pack/judgment-pack-agent", + "run_id": 41149914554, + "pull_request": 267, + "tag_repository": "Judgment-Pack/judgment-pack-devkit", + "tag": "v0.13.0", + "note": "Read-only API projections; omitted unrelated fields and identities. Not generated by the gate.", + "fields": { + "repository": "id full_name default_branch", + "commit": "sha tree.sha parents[].sha", + "commit-pulls": "[].number [].state [].created_at [].merged_at [].head.sha [].head.ref [].head.repo.id [].base.ref [].base.repo.id", + "head-pulls": "[].number [].head.repo.id", + "reported-pull": "number state created_at merged_at head.sha head.ref head.repo.id base.ref base.repo.id", + "timeline": "[].event", + "workflow": "id path name state", + "reported-run": "id run_attempt event head_branch head_sha workflow_id path status conclusion pull_requests[].number pull_requests[].base.ref pull_requests[].base.repo.id created_at run_started_at updated_at repository.id repository.full_name head_repository.id head_repository.full_name", + "jobs": "total_count jobs[].id jobs[].name jobs[].run_id jobs[].run_attempt jobs[].head_sha jobs[].head_branch jobs[].workflow_name jobs[].status jobs[].conclusion jobs[].labels jobs[].runner_group_id jobs[].runner_group_name jobs[].started_at jobs[].completed_at", + "artifacts": "total_count artifacts[].id artifacts[].name artifacts[].expired artifacts[].size_in_bytes artifacts[].created_at artifacts[].digest artifacts[].workflow_run.id artifacts[].workflow_run.head_sha artifacts[].workflow_run.head_branch artifacts[].workflow_run.repository_id artifacts[].workflow_run.head_repository_id", + "tag-ref": "ref object.type object.sha", + "tag-object": "object.type object.sha" + }, + "samples": { + "repository": { + "id": 1424951648, + "full_name": "Judgment-Pack/judgment-pack-agent", + "default_branch": "main" + }, + "commit": { + "sha": "52441d17bc33b9a0346047bff02b469db93dc84d", + "tree": { + "sha": "cef1c96abf53ea9f1ff2ee017a081f4aead2cb59" + }, + "parents": [ + { + "sha": "8b74d3f0dbc4815927d579642c9bdd37d8748bef" + } + ] + }, + "commit-pulls": [ + { + "number": 267, + "state": "closed", + "created_at": "2026-10-30T19:23:08Z", + "merged_at": "2026-10-31T00:23:33Z", + "head": { + "sha": "4d269769d0be2659a1cac7e85cbaab503c7c8982", + "ref": "ci-identity-reuse-239-240", + "repo": { + "id": 1424951648 + } + }, + "base": { + "ref": "main", + "repo": { + "id": 1424951648 + } + } + } + ], + "head-pulls": [ + { + "number": 267, + "head": { + "repo": { + "id": 1424951648 + } + } + } + ], + "reported-pull": { + "number": 267, + "state": "closed", + "created_at": "2026-10-30T19:23:08Z", + "merged_at": "2026-10-31T00:23:33Z", + "head": { + "sha": "4d269769d0be2659a1cac7e85cbaab503c7c8982", + "ref": "ci-identity-reuse-239-240", + "repo": { + "id": 1424951648 + } + }, + "base": { + "ref": "main", + "repo": { + "id": 1424951648 + } + } + }, + "timeline": [ + { + "event": "committed" + }, + { + "event": "milestoned", + "commit_id": null + }, + { + "event": "commented" + }, + { + "event": "cross-referenced" + }, + { + "event": "committed" + }, + { + "event": "commented" + }, + { + "event": "committed" + }, + { + "event": "committed" + }, + { + "event": "merged", + "commit_id": "52441d17bc33b9a0346047bff02b469db93dc84d" + }, + { + "event": "closed", + "commit_id": null + }, + { + "event": "head_ref_deleted", + "commit_id": null + }, + { + "event": "commented" + } + ], + "workflow": { + "id": 388881851, + "path": ".github/workflows/ci.yml", + "name": "CI", + "state": "active" + }, + "reported-run": { + "id": 41149914554, + "run_attempt": 1, + "event": "push", + "head_branch": "main", + "head_sha": "52441d17bc33b9a0346047bff02b469db93dc84d", + "workflow_id": 388881851, + "path": ".github/workflows/ci.yml", + "status": "completed", + "conclusion": "success", + "pull_requests": [], + "created_at": "2026-10-31T00:23:36Z", + "run_started_at": "2026-10-31T00:23:36Z", + "updated_at": "2026-10-31T00:58:23Z", + "repository": { + "id": 1424951648, + "full_name": "Judgment-Pack/judgment-pack-agent" + }, + "head_repository": { + "id": 1424951648, + "full_name": "Judgment-Pack/judgment-pack-agent" + } + }, + "jobs": { + "total_count": 2, + "jobs": [ + { + "id": 124383723937, + "name": "contract", + "run_id": 41149914554, + "run_attempt": 1, + "head_sha": "52441d17bc33b9a0346047bff02b469db93dc84d", + "head_branch": "main", + "workflow_name": "CI", + "status": "completed", + "conclusion": "success", + "labels": [ + "ubuntu-latest" + ], + "runner_group_id": 0, + "runner_group_name": "GitHub Actions", + "started_at": "2026-10-31T00:23:38Z", + "completed_at": "2026-10-31T00:23:47Z" + }, + { + "id": 124383724097, + "name": "check", + "run_id": 41149914554, + "run_attempt": 1, + "head_sha": "52441d17bc33b9a0346047bff02b469db93dc84d", + "head_branch": "main", + "workflow_name": "CI", + "status": "completed", + "conclusion": "success", + "labels": [ + "ubuntu-latest" + ], + "runner_group_id": 0, + "runner_group_name": "GitHub Actions", + "started_at": "2026-10-31T00:23:38Z", + "completed_at": "2026-10-31T00:58:23Z" + } + ] + }, + "artifacts": { + "total_count": 2, + "artifacts": [ + { + "id": 13023513896, + "name": "ci-evidence-ci-check-1", + "expired": false, + "size_in_bytes": 2664, + "created_at": "2026-10-31T00:58:19Z", + "digest": "sha256:e20e6f51dc8884906da0937cb0016c53a48a51a0f0c9266e832fb5f86ed85d3a", + "workflow_run": { + "id": 41149914554, + "head_sha": "52441d17bc33b9a0346047bff02b469db93dc84d", + "head_branch": "main", + "repository_id": 1424951648, + "head_repository_id": 1424951648 + } + }, + { + "id": 13022233372, + "name": "ci-evidence-ci-contract-1", + "expired": false, + "size_in_bytes": 2213, + "created_at": "2026-10-31T00:23:45Z", + "digest": "sha256:cc9ff0c74cec9fcdd09cdf10c4f69cb8a9e3e087c2e93d5088d3191a9bf1f465", + "workflow_run": { + "id": 41149914554, + "head_sha": "52441d17bc33b9a0346047bff02b469db93dc84d", + "head_branch": "main", + "repository_id": 1424951648, + "head_repository_id": 1424951648 + } + } + ] + }, + "tag-ref": { + "ref": "refs/tags/v0.13.0", + "object": { + "type": "tag", + "sha": "c5747eb99985b1d1f451313ac2c8d52843e2bdee" + } + }, + "tag-object": { + "object": { + "type": "commit", + "sha": "939b0377e57e1d09964e3b4d5dc4c45ad70c7854" + } + }, + "runs": { + "total_count": 1, + "workflow_runs": [ + { + "id": 41149914554, + "run_attempt": 1, + "event": "push", + "head_branch": "main", + "head_sha": "52441d17bc33b9a0346047bff02b469db93dc84d", + "workflow_id": 388881851, + "path": ".github/workflows/ci.yml", + "status": "completed", + "conclusion": "success", + "pull_requests": [], + "created_at": "2026-10-31T00:23:36Z", + "run_started_at": "2026-10-31T00:23:36Z", + "updated_at": "2026-10-31T00:58:23Z", + "repository": { + "id": 1424951648, + "full_name": "Judgment-Pack/judgment-pack-agent" + }, + "head_repository": { + "id": 1424951648, + "full_name": "Judgment-Pack/judgment-pack-agent" + } + } + ] + }, + "pr-runs": { + "total_count": 1, + "workflow_runs": [ + { + "id": 41145567567, + "run_attempt": 1, + "event": "pull_request", + "head_branch": "ci-identity-reuse-239-240", + "head_sha": "4d269769d0be2659a1cac7e85cbaab503c7c8982", + "workflow_id": 388881851, + "path": ".github/workflows/ci.yml", + "status": "completed", + "conclusion": "success", + "pull_requests": [], + "created_at": "2026-10-30T23:37:22Z", + "run_started_at": "2026-10-30T23:37:22Z", + "updated_at": "2026-10-31T00:23:23Z", + "repository": { + "id": 1424951648, + "full_name": "Judgment-Pack/judgment-pack-agent" + }, + "head_repository": { + "id": 1424951648, + "full_name": "Judgment-Pack/judgment-pack-agent" + } + } + ] + }, + "nonempty-run-association": { + "id": 41223435315, + "pull_requests": [ + { + "base": { + "ref": "principals-260", + "repo": { + "id": 1424951648 + } + }, + "number": 276 + } + ] + } + } +} diff --git a/tests/release_evidence_test.py b/tests/release_evidence_test.py index d200d91..dd8896e 100644 --- a/tests/release_evidence_test.py +++ b/tests/release_evidence_test.py @@ -1,6 +1,6 @@ """The release evidence gate over handwritten Actions API fixtures; none of this is live evidence. -Every fixture below is written by hand. The refusal classes of docs/ci-evidence.md each have a test +Decision fixtures below are written by hand; PinnedAPIShape separately reads captured API projections. The refusal classes of docs/ci-evidence.md each have a test class here; the module is lib/release_evidence.py unless RELEASE_EVIDENCE_HELPER names a copy (a scratch mutant, the vendored scripts/dev/release_evidence.py). """ @@ -2190,7 +2190,7 @@ def test_capture_refuses_an_unexpected_identity(self): TREE = '7' * 40 # the commit's tree, and the merge commit's GOFLAGS = hashlib.sha256(b'-count=1').hexdigest() PULL = {'number': 7, 'state': 'closed', 'created_at': '2026-10-25T08:30:00Z', 'merged_at': '2026-10-25T09:55:00Z', - 'merge_commit_sha': SHA, 'head': {'sha': HEAD, 'ref': 'feature', 'repo': {'id': REPO_ID}}, + 'head': {'sha': HEAD, 'ref': 'feature', 'repo': {'id': REPO_ID}}, 'base': {'ref': 'main', 'repo': {'id': REPO_ID}}} PR_RUNS = 'actions/workflows/9/runs?head_sha=' + HEAD + '&per_page=100&page=1' PR_JOBS = 'actions/runs/50/attempts/1/jobs?per_page=100&page=1' @@ -2216,6 +2216,33 @@ def test_capture_refuses_an_unexpected_identity(self): environment={'GOFLAGS': GOFLAGS}) +class PinnedAPIShape(unittest.TestCase): + """Read-only captured API projections, separate from handwritten decision vectors.""" + + def test_captured_merge_binding_without_obsolete_field(self): + fixture = json.loads((ROOT / 'tests/fixture/github-api-2026-03-10.json').read_text()) + sample = fixture['samples'] + self.assertEqual(fixture['api_version'], '2026-03-10') + sha = sample['commit']['sha'] + pull = sample['commit-pulls'][0] + self.assertNotIn('merge_commit_sha', pull) + self.assertNotIn('merge_commit_sha', sample['reported-pull']) + source = {'source': sha, 'repository': fixture['repository'], + 'repository_id': sample['repository']['id']} + head_listing = 'pulls?head=Judgment-Pack:%s&state=all&per_page=100&page=' % pull['head']['ref'] + timeline = 'issues/%d/timeline?per_page=100&page=' % pull['number'] + data = {'git/commits/' + sha: sample['commit'], + 'commits/%s/pulls?per_page=100' % sha: sample['commit-pulls'], + head_listing + '1': sample['head-pulls'], head_listing + '2': [], + timeline + '1': sample['timeline'], timeline + '2': []} + bound, commit = e.pull_request_of(FakeAPI(data), {'branch': 'main'}, source) + self.assertEqual((bound['number'], commit['sha']), (267, sha)) + data[timeline + '1'] = [v for v in sample['timeline'] if v.get('event') != 'merged'] + with self.assertRaises(e.Refusal) as caught: + e.pull_request_of(FakeAPI(data), {'branch': 'main'}, source) + self.assertEqual(caught.exception.code, 'reuse-pull-request') + + class ReuseFixtures(Gate): """A branch run whose Linux job took `unit` from the merged pull request's run.""" @@ -2228,7 +2255,7 @@ def setUp(self): 'git/commits/' + MERGE: {'sha': MERGE, 'tree': {'sha': TREE}, 'parents': [{'sha': BASE}, {'sha': HEAD}]}, 'commits/%s/pulls?per_page=100' % SHA: [PULL], HEAD_PULLS: [PULL], HEAD_PULLS_END: [], - TIMELINE: [{'event': 'committed'}, {'event': 'reviewed'}, {'event': 'labeled'}, {'event': 'merged'}, + TIMELINE: [{'event': 'committed'}, {'event': 'reviewed'}, {'event': 'labeled'}, {'event': 'merged', 'commit_id': SHA}, {'event': 'head_ref_deleted'}], TIMELINE_END: [], PR_RUNS: {'total_count': 1, 'workflow_runs': [PR_RUN]}, @@ -2270,7 +2297,7 @@ def foreign_run(self): self.pr_runs(own, foreign) self.data['actions/runs/50'] = own self.data['actions/runs/60'] = foreign - self.data[HEAD_PULLS] = [PULL, dict(PULL, number=8, state='open', merged_at=None, merge_commit_sha=None, + self.data[HEAD_PULLS] = [PULL, dict(PULL, number=8, state='open', merged_at=None, base=dict(PULL['base'], ref='evil'))] self.data['actions/runs/60/attempts/1/jobs?per_page=100&page=1'] = { 'total_count': 1, 'jobs': [dict(PR_JOB, id=161, run_id=60, started_at='2026-10-25T09:20:05Z', @@ -2399,7 +2426,7 @@ def test_every_base_change_event_refuses(self): def test_listings_are_read_to_an_empty_page(self): # A short first page is not the end: a base change on the next page refuses. - self.data[TIMELINE] = [{'event': 'committed'}] * 30 + self.data[TIMELINE] = [{'event': 'merged', 'commit_id': SHA}] + [{'event': 'committed'}] * 30 self.data[TIMELINE_END] = [{'event': 'automatic_base_change_succeeded'}] self.data['issues/7/timeline?per_page=100&page=3'] = [] self.assertEqual(self.decide(), 'reuse-pull-request') @@ -2430,7 +2457,7 @@ def test_the_pull_request_job_runner_and_the_merge_commit_parent_order(self): def test_the_trusted_pull_request_relationship(self): for change in (lambda: self.data['commits/%s/pulls?per_page=100' % SHA].clear(), - lambda: self.data['commits/%s/pulls?per_page=100' % SHA][0].update(merge_commit_sha=OTHER), + lambda: self.data[TIMELINE][3].update(commit_id=OTHER), lambda: self.data['commits/%s/pulls?per_page=100' % SHA][0].update(merged_at=None), lambda: self.data['commits/%s/pulls?per_page=100' % SHA].append(dict(PULL, number=8)), lambda: self.data['commits/%s/pulls?per_page=100' % SHA][0]['base'].update(ref='release'), @@ -2624,6 +2651,38 @@ def test_a_plan_reuses_what_the_pull_request_run_passed(self): self.assertEqual((self.record['covered'], self.record['reused']), (['unit'], self.reused)) self.assertEqual(self.record['attempt'], 1) + def test_pinned_api_shape_uses_the_merge_event_not_a_pull_field(self): + pull = self.data['commits/%s/pulls?per_page=100' % SHA][0] + self.assertNotIn('merge_commit_sha', pull) + self.assertEqual(self.plan(), 'reuse') + # Even an obsolete field cannot override the trusted timeline binding. + pull['merge_commit_sha'] = OTHER + self.assertEqual(self.plan(), 'reuse') + + def test_missing_ambiguous_or_malformed_merge_events_refuse_by_name(self): + for events in ([], [{'event': 'merged'}], [{'event': 'merged', 'commit_id': None}], + [{'event': 'merged', 'commit_id': 'invalid'}], + [{'event': 'merged', 'commit_id': OTHER}], + [{'event': 'merged', 'commit_id': SHA}] * 2, + [{'event': 'merged', 'commit_id': SHA}, + {'event': 'merged', 'commit_id': OTHER}]): + with self.subTest(events=events): + self.setUp() + self.data[TIMELINE] = events + self.assertEqual(self.plan(), 'reuse-pull-request') + self.assertEqual(self.record['covered'], []) + + def test_merge_event_on_a_later_page_and_unrelated_merged_pull(self): + self.data[TIMELINE] = [{'event': 'reviewed'}] + self.data[TIMELINE_END] = [{'event': 'merged', 'commit_id': SHA}] + self.data['issues/7/timeline?per_page=100&page=3'] = [] + self.data['commits/%s/pulls?per_page=100' % SHA].append(dict(PULL, number=8)) + self.data['issues/8/timeline?per_page=100&page=1'] = [{'event': 'merged', 'commit_id': OTHER}] + self.data['issues/8/timeline?per_page=100&page=2'] = [] + self.assertEqual(self.plan(), 'reuse') + self.data['issues/8/timeline?per_page=100&page=1'][0]['commit_id'] = SHA + self.assertEqual(self.plan(), 'reuse-pull-request') + def test_a_plan_never_takes_another_pull_requests_run(self): # Review probe P1 at the plan: fresh, and the job runs its whole list. self.foreign_run() @@ -2810,9 +2869,9 @@ def iso(seconds): data = {'git/commits/' + self.sha: {'sha': self.sha, 'tree': {'sha': self.tree}, 'parents': [{'sha': self.base}]}, 'git/commits/' + self.merge: {'sha': self.merge, 'tree': {'sha': self.tree}, 'parents': [{'sha': self.base}, {'sha': self.head}]}, - HEAD_PULLS: [dict(PULL, merge_commit_sha=self.sha, head=dict(PULL['head'], sha=self.head))], - HEAD_PULLS_END: [], TIMELINE: [], TIMELINE_END: [], - 'commits/%s/pulls?per_page=100' % self.sha: [dict(PULL, merge_commit_sha=self.sha, + HEAD_PULLS: [dict(PULL, head=dict(PULL['head'], sha=self.head))], + HEAD_PULLS_END: [], TIMELINE: [{'event': 'merged', 'commit_id': self.sha}], TIMELINE_END: [], + 'commits/%s/pulls?per_page=100' % self.sha: [dict(PULL, head=dict(PULL['head'], sha=self.head))], 'actions/workflows/ci.yml': {'id': 9, 'name': 'CI', 'path': '.github/workflows/ci.yml', 'state': 'active'}, 'actions/workflows/9/runs?head_sha=%s&per_page=100&page=1' % self.head: diff --git a/tests/run.sh b/tests/run.sh index 49befac..6acb8ea 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -41,8 +41,8 @@ absent() { if grep -qE -- "$2" <<< "$3"; then bad "$1 (unexpected line matching: $2)" "$3"; else ok "$1"; fi } -# The release evidence gate (lib/release_evidence.py): handwritten API fixtures, never live-service -# evidence. Each refusal class of docs/ci-evidence.md, the decision record and the public command. +# The release evidence gate (lib/release_evidence.py): handwritten decision fixtures plus +# read-only pinned API shape projections, never evidence of a live check run. Each refusal class of docs/ci-evidence.md, the decision record and the public command. out=$(python3 "$kit/tests/release_evidence_test.py" 2>&1) && s=0 || s=$? expect "release evidence gate: exact-commit reuse and every refusal class over API fixtures" 0 'OK' "$out" "$s" From 4a99e4e0ed3275913319d41ec95df99badbe4f7f Mon Sep 17 00:00:00 2001 From: kikashy Date: Sun, 1 Nov 2026 08:39:07 -0500 Subject: [PATCH 2/2] Account for nested process groups in the RSS guard Signed-off-by: kikashy --- README.md | 26 ++++++++- config/env.example | 3 +- lib/rsscap.sh | 71 ++++++++++++++++++----- tests/rsscap_test.py | 135 +++++++++++++++++++++++++++++++++++++++++++ tests/run.sh | 4 ++ 5 files changed, 222 insertions(+), 17 deletions(-) create mode 100644 tests/rsscap_test.py diff --git a/README.md b/README.md index 60ad630..2e06357 100644 --- a/README.md +++ b/README.md @@ -256,11 +256,35 @@ duplication.min_tokens: an integer from 10 to 10000 is required (got 5)`. | Toolchain | Cap | | --- | --- | -| `node` | `lib/rsscap.sh`: the process group's resident memory and a timeout; never `ulimit -v`, which breaks vite and vitest | +| `node` | `lib/rsscap.sh`: the launched session's sampled resident memory and a timeout; never `ulimit -v`, which breaks vite and vitest | | `go` | `flock` on the machine lock, `ulimit -v`, `GOFLAGS=-mod=readonly` (a step never rewrites `go.mod`), a per-clone `GOCACHE`, `timeout` | | `python` | `ulimit -v` and `timeout` | | `shell` | `timeout` only | +The RSS guard counts all non-zombie processes in the new session, including GNU +`timeout` and compiler/test children in other process groups. It keeps monitoring +until the session is empty, even when the original command exits first. Memory, +time and interruption stops kill the session's observed process groups. The +command's exit status is preserved when no guard stops it. Limits are positive +integers of at most nine digits; RSS is compared in KiB against the MiB limit. + +This is a cooperative, sampled guard, not a security boundary or a hard memory +reservation. Between samples a process may exceed the limit. Descendants that +call `setsid` leave the owned session and are neither measured nor cleaned up; +use an external containment mechanism for workloads that detach this way. Shared +pages can be counted more than once when summing resident memory. + +For a Go race check that cannot use the virtual-address limit, retain the existing +wrapper's lock, per-clone cache, read-only module mode and inner timeout: + +```sh +JPDEV_GO_ULIMIT_KB=unlimited lib/rsscap.sh 4096 300 -- lib/wrap.sh go -- go test -race ./... +``` + +This explicit invocation applies the outer RSS/time limits to the wrapper and its +session. Normal Go steps retain their existing virtual-address cap. A limit that +fires while the wrapper waits for the lock also stops that wait. + When the manifest declares a virtual environment, its path must be a non-root relative child of the clone with no `..` segment, and setup and checks create it when it is missing. They use `python3.X` on PATH first, then the newest `~/.pyenv/versions/3.X.*/bin/python3`, and diff --git a/config/env.example b/config/env.example index b9bd939..4c36898 100644 --- a/config/env.example +++ b/config/env.example @@ -11,11 +11,12 @@ # The machine lock every Go step holds, so two sessions never run Go at once. #JPDEV_LOCK="$JPDEV_SCRATCH/harness.lock" -# Node steps: the resident-memory cap of the step's process group (MB) and its default timeout (s). +# Node steps: the resident-memory cap of the step's session (MiB) and its default timeout (s). #JPDEV_NODE_RSS_MB=6000 #JPDEV_NODE_TIMEOUT_S=1200 # Go steps: ulimit -v (KB) and the default timeout (s). The per-clone GOCACHE sits under JPDEV_SCRATCH. +# Set unlimited only with an explicit outer RSS guard for a race check; see README.md. #JPDEV_GO_ULIMIT_KB=12000000 #JPDEV_GO_TIMEOUT_S=3000 # Go steps always run with GOFLAGS=-mod=readonly and the per-clone GOCACHE, which lib/wrap.sh sets diff --git a/lib/rsscap.sh b/lib/rsscap.sh index 472ac43..cd7ef96 100755 --- a/lib/rsscap.sh +++ b/lib/rsscap.sh @@ -1,29 +1,70 @@ #!/usr/bin/env bash # rsscap.sh MAX_MB TIMEOUT_S -- command... -# Runs the command in its own process group and stops the whole group when the -# group's resident memory passes MAX_MB or the run passes TIMEOUT_S. -# For programs an address-space cap (ulimit -v) breaks, such as vite and vitest. -# Exit: the command's own status; 137 when stopped for memory; 124 for time. -set -u -if ! { [ $# -ge 4 ] && [ "$3" = "--" ]; }; then echo "usage: rsscap.sh MAX_MB TIMEOUT_S -- command..." >&2; exit 2; fi +# Sample resident memory across the command's new session, including nested process +# groups (for example GNU timeout). A descendant that creates a new session escapes +# this cooperative guard; it is not a security boundary or an instantaneous limit. +# Exit: command status; 137 for memory; 124 for time; 2 for invalid/unreadable input. +set -uo pipefail +if ! { [ $# -ge 4 ] && [ "$3" = "--" ] && [[ "$1" =~ ^[1-9][0-9]{0,8}$ ]] && [[ "$2" =~ ^[1-9][0-9]{0,8}$ ]]; }; then + echo "usage: rsscap.sh MAX_MB TIMEOUT_S -- command... (positive integers, at most nine digits)" >&2 + exit 2 +fi max_mb=$1; tmo=$2; shift 3 +# No job-control process group for the background child: setsid keeps this PID as SID. +set +m setsid "$@" & pid=$! -start=$(date +%s) +start=$SECONDS peak=0 -while kill -0 "$pid" 2>/dev/null; do - rss_mb=$(ps -eo pgid=,rss= | awk -v g="$pid" '$1==g {s+=$2} END {printf "%d", s/1024}') + +session_rows() { + ps -eo sid=,pgid=,stat=,rss= | awk -v session="$pid" '$1 == session && $3 !~ /^Z/ {print $2, $4}' +} + +# Invoked by the EXIT trap, including signal and limit exits. +# shellcheck disable=SC2329 +cleanup() { + local rows groups group round + trap '' HUP INT TERM + # The launcher may not have entered its session yet. Stop it as well as every + # observed group; never select by process name or another session's ancestry. + if jobs -pr | grep -qx "$pid"; then kill -KILL "$pid" 2>/dev/null || true; fi + for ((round=0; round<10; round++)); do + rows=$(session_rows) || { echo 'rsscap: could not read session during cleanup' >&2; exit 2; } + [ -n "$rows" ] || break + groups=$(awk '{print $1}' <<< "$rows" | sort -u) + while read -r group; do + kill -KILL -- "-$group" 2>/dev/null || true + done <<< "$groups" + sleep 0.05 + done + rows=$(session_rows) || { echo 'rsscap: could not verify session cleanup' >&2; exit 2; } + if [ -n "$rows" ]; then echo 'rsscap: session cleanup did not finish' >&2; exit 2; fi + wait "$pid" 2>/dev/null || true +} +trap 'cleanup' EXIT +trap 'exit 129' HUP +trap 'exit 130' INT +trap 'exit 143' TERM + +while :; do + rows=$(session_rows) || { echo 'rsscap: could not read session memory' >&2; exit 2; } + # Keep watching after the leader exits: a background child still belongs to the + # session, even after reparenting or moving to a different process group. + if [ -z "$rows" ] && ! kill -0 "$pid" 2>/dev/null; then break; fi + rss_kb=$(awk '{s += $2} END {printf "%.0f", s}' <<< "$rows") + rss_mb=$(( (rss_kb + 1023) / 1024 )) [ "$rss_mb" -gt "$peak" ] && peak=$rss_mb - if [ "$rss_mb" -gt "$max_mb" ]; then - echo "rsscap: STOPPED for memory: the group held ${rss_mb} MB, over the ${max_mb} MB limit" >&2 - kill -KILL -- "-$pid" 2>/dev/null; wait "$pid" 2>/dev/null; exit 137 + if [ "$rss_kb" -gt $(( max_mb * 1024 )) ]; then + echo "rsscap: STOPPED for memory: the session held ${rss_mb} MB, over the ${max_mb} MB limit" >&2 + exit 137 fi - if [ $(( $(date +%s) - start )) -gt "$tmo" ]; then + if [ $(( SECONDS - start )) -ge "$tmo" ]; then echo "rsscap: STOPPED for time: over ${tmo} s (peak ${peak} MB)" >&2 - kill -KILL -- "-$pid" 2>/dev/null; wait "$pid" 2>/dev/null; exit 124 + exit 124 fi sleep 0.5 done wait "$pid"; status=$? echo "rsscap: exit ${status}, peak ${peak} MB" >&2 -exit $status +exit "$status" diff --git a/tests/rsscap_test.py b/tests/rsscap_test.py new file mode 100644 index 0000000..9254bde --- /dev/null +++ b/tests/rsscap_test.py @@ -0,0 +1,135 @@ +"""Actual RSS guard/wrapper entrypoints; bounded cooperative process-session probes.""" +import os +from pathlib import Path +import signal +import subprocess +import sys +import tempfile +import time +import unittest + +ROOT = Path(__file__).resolve().parents[1] +GUARD = Path(os.environ.get('RSSCAP_HELPER', ROOT / 'lib/rsscap.sh')) + + +class SessionCap(unittest.TestCase): + def run_guard(self, command, memory=24, seconds=6, env=None): + # Files, not pipes: an escaped child retaining stdout must not delay the + # parent's wait until it exits naturally and conceal failed cleanup. + with tempfile.TemporaryFile(mode='w+') as out, tempfile.TemporaryFile(mode='w+') as err: + result = subprocess.run([str(GUARD), str(memory), str(seconds), '--', *command], + stdout=out, stderr=err, timeout=12, env=env) + out.seek(0) + err.seek(0) + result.stdout, result.stderr = out.read(), err.read() + return result + + def assert_stopped(self, result, code): + for line in result.stdout.splitlines(): + if line.startswith('child='): + pid = int(line.split('=')[1]) + # An orphan can be a zombie until the system reaps it. It must not run. + state = subprocess.run(['ps', '-o', 'stat=', '-p', str(pid)], + capture_output=True, text=True, check=False).stdout.strip() + stopped = not state or state.startswith('Z') + if not stopped: + # Cleanup even a deliberately broken guard's bounded probe. + try: + os.kill(pid, signal.SIGKILL) + except ProcessLookupError: + pass + self.assertTrue(stopped, (pid, state)) + self.assertEqual(result.returncode, code, result.stderr) + + def test_nested_timeout_memory_and_cleanup(self): + probe = ('import os,time; print("separate="+str(os.getpgrp()!=os.getsid(0)),flush=True); print("child="+str(os.getpid()),flush=True); ' + 'held=bytearray(64*1024*1024); time.sleep(2)') + result = self.run_guard(['bash', '-c', '"$@"; code=$?; exit "$code"', + 'probe', 'timeout', '-k', '1', '5', sys.executable, '-c', probe]) + self.assert_stopped(result, 137) + self.assertIn('STOPPED for memory', result.stderr) + self.assertIn('separate=True', result.stdout) + + def test_nested_timeout_wall_limit_and_cleanup(self): + probe = 'import os,time; print("child="+str(os.getpid()),flush=True); time.sleep(3)' + result = self.run_guard(['bash', '-c', '"$@"; code=$?; exit "$code"', + 'probe', 'timeout', '5', sys.executable, '-c', probe], memory=256, seconds=1) + self.assert_stopped(result, 124) + + def test_session_survives_its_leader(self): + probe = ('import os,time; print("child="+str(os.getpid()),flush=True); ' + 'held=bytearray(64*1024*1024); time.sleep(2)') + result = self.run_guard(['bash', '-c', 'timeout 5 "$@" & exit 7', 'probe', sys.executable, '-c', probe]) + self.assert_stopped(result, 137) + + def test_memory_is_summed_across_groups(self): + probe = ('import os,time; print("child="+str(os.getpid()),flush=True); ' + 'held=bytearray(16*1024*1024); time.sleep(2)') + parent = ('import subprocess,sys; children=[subprocess.Popen(["timeout","5",sys.executable,"-c",%r]) ' + 'for _ in range(2)]; [p.wait() for p in children]') % probe + self.assert_stopped(self.run_guard([sys.executable, '-c', parent], memory=40), 137) + + def test_new_session_is_an_explicit_limit(self): + # Cooperative guard only: a detached child is outside its accounting. Wait + # for that child here so even this limitation probe leaves no live process. + probe = 'import time; held=bytearray(64*1024*1024); time.sleep(1)' + parent = ('import subprocess,sys; p=subprocess.Popen(["setsid",sys.executable,"-c",%r]); ' + 'raise SystemExit(p.wait())') % probe + self.assertEqual(self.run_guard([sys.executable, '-c', parent]).returncode, 0) + + def test_status_and_invalid_limits(self): + self.assertEqual(self.run_guard(['bash', '-c', 'exit 7']).returncode, 7) + self.assertEqual(self.run_guard(['true'], memory=0).returncode, 2) + self.assertEqual(self.run_guard(['true'], seconds=-1).returncode, 2) + + def test_signal_cleans_other_process_groups(self): + with tempfile.TemporaryDirectory(prefix='rsscap-signal-') as folder: + pidfile = Path(folder) / 'child' + probe = ('import os,time,pathlib; pathlib.Path(%r).write_text(str(os.getpid())); ' + 'time.sleep(4)') % str(pidfile) + errfile = tempfile.TemporaryFile(mode='w+') + self.addCleanup(errfile.close) + process = subprocess.Popen([str(GUARD), '256', '8', '--', 'bash', '-c', '"$@"; code=$?; exit "$code"', 'probe', 'timeout', '6', + sys.executable, '-c', probe], stdout=subprocess.DEVNULL, + stderr=errfile, text=True) + try: + deadline = time.monotonic() + 3 + while not pidfile.exists() and time.monotonic() < deadline: + time.sleep(0.02) + self.assertTrue(pidfile.exists(), 'child did not start') + process.send_signal(signal.SIGTERM) + process.wait(timeout=8) + errfile.seek(0) + stderr = errfile.read() + result = subprocess.CompletedProcess([], process.returncode, + 'child=' + pidfile.read_text(), stderr) + self.assert_stopped(result, 143) + finally: + if process.poll() is None: + process.kill() + process.communicate(timeout=8) + + def test_go_wrapper_keeps_lock_cache_readonly_and_removes_vm_cap(self): + with tempfile.TemporaryDirectory(prefix='rsscap-wrapper-') as folder: + env = dict(os.environ, JPDEV_SCRATCH=folder, JPDEV_LOCK=folder + '/heavy.lock', + XDG_CONFIG_HOME=folder, JPDEV_GO_ULIMIT_KB='unlimited', + JPDEV_STEP_TIMEOUT='5', JPDEV_ROOT=str(ROOT)) + script = ''' + test "$GOFLAGS" = -mod=readonly || exit 90 + case "$GOCACHE" in "$JPDEV_SCRATCH"/gocache/*) ;; *) exit 91;; esac + test "$(ulimit -v)" = unlimited || exit 92 + if flock -n "$JPDEV_LOCK" true; then exit 93; fi + exec "$@" + ''' + probe = ('import os,time; print("child="+str(os.getpid()),flush=True); ' + 'held=bytearray(64*1024*1024); time.sleep(2)') + command = [str(ROOT / 'lib/wrap.sh'), 'go', '--', 'bash', '-c', script, + 'probe', sys.executable, '-c', probe] + result = self.run_guard(command, env=env) + self.assert_stopped(result, 137) + unlocked = subprocess.run(['flock', '-n', env['JPDEV_LOCK'], 'true'], check=False) + self.assertEqual(unlocked.returncode, 0) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/run.sh b/tests/run.sh index 6acb8ea..2df4eb4 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -65,6 +65,10 @@ expect "jpdev check --changed selects the mapped checks of fixture clones" 0 'OK out=$(python3 "$kit/tests/check_incomplete_test.py" 2>&1) && s=0 || s=$? expect "check.sh incomplete-checklist status and completed-step controls" 0 'OK' "$out" "$s" +# Real RSS guard and Go-wrapper entrypoints, including separate timeout process groups. +out=$(python3 "$kit/tests/rsscap_test.py" 2>&1) && s=0 || s=$? +expect "rsscap owns nested process groups until exit and preserves Go wrapper controls" 0 'OK' "$out" "$s" + # --- the public runner, with no caps out=$("$kit/templates/check.sh" --manifest "$fixture" 2>/dev/null) && s=0 || s=$? # JPDEV_ROOT is unset: the template's own location is the kit, whose git root is this repository.