From 3ee8a700e8cff64dda483d61a902437609f3d7bb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 19:16:51 +0900 Subject: [PATCH 01/14] fix(scheduler): isolate central Actions inventory quota --- CHANGELOG.md | 12 ++++----- docs/doctoring/fork-head-review-dispatch.md | 10 ++++++++ scripts/ci/pr_review_merge_scheduler.py | 12 +++++++-- tests/test_pr_review_merge_scheduler.py | 28 +++++++++++++++++++++ 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47c14f765a..a126924d32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,12 +45,12 @@ Semantic Versioning where the repository publishes a release. ### Fixed -- Used the receiving repository's workflow token for same-repository scheduler - Actions inventory and read calls, while retaining the established mutation - credential chain. An exhausted organization-wide OpenCode App installation - budget can no longer prevent a central `.github` PR from dispatching its - exact-head review; cross-repository targets still require an explicit - credential. +- Use the repository hosting each workflow run to select scheduler Actions + credentials: central required-workflow inventory uses the receiving + repository's job token, while target-repository inventory and mutations keep + the established explicit credential chain. An exhausted organization-wide + OpenCode App installation budget can no longer prevent either central or + cross-repository PRs from dispatching exact-head review. - Kept independently valid root-level Python lock environments separate during trusted base coverage installation. A directory with more than two candidate locks no longer collapses unrelated OpenCode, security, and application diff --git a/docs/doctoring/fork-head-review-dispatch.md b/docs/doctoring/fork-head-review-dispatch.md index fe326459f8..c854c2563b 100644 --- a/docs/doctoring/fork-head-review-dispatch.md +++ b/docs/doctoring/fork-head-review-dispatch.md @@ -65,6 +65,16 @@ distinguish a same-repository target from a cross-repository target. The full Python suite, 100% statement/branch/docstring gates, and the CI-budget Strix shell gate remain authoritative before publication. +Targeted cross-repository run `32566396712` later exposed the remaining host +boundary: while reviewing `contextual-orchestrator#820`, active OpenCode run +discovery queried the central `.github` Actions inventory with the shared App +token and exhausted that installation's quota before dispatch. Active-run +inventory now selects credentials by the repository hosting the run. Central +required-workflow runs use the receiving repository's job token; target-repo +run inventory and mutations retain the explicit cross-repository credential. +The regression exercises both hosts in one call sequence so a later refactor +cannot collapse them back onto one rate-limit bucket. + ## APA 7th references GitHub, Inc. (n.d.-a). *REST API endpoints for pull requests*. GitHub Docs. diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 44620fcab5..8734d1de72 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1922,11 +1922,19 @@ def rerun_actions_job(repo: str, job_id: str, *, dry_run: bool, action: str) -> def active_workflow_runs(repo: str, statuses: Sequence[str] = ("queued", "in_progress")) -> list[dict[str, Any]]: - """Return active workflow runs for a repository.""" + """Return active workflow runs with the credential scoped to their host.""" runs: list[dict[str, Any]] = [] + central_repo = ( + os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "" + ).strip() + run_command = ( + run_github_dispatch + if central_repo and repo == central_repo + else run_github_actions + ) for status in statuses: payload = json.loads( - run_github_actions( + run_command( [ "gh", "api", diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 0e71bdbe24..0c65b001eb 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1967,6 +1967,34 @@ def fake_run_with_env(args, *, stdin=None, env=None): ] +def test_active_workflow_runs_use_central_runner_token_for_central_dispatch( + monkeypatch, +): + """Central run discovery must not spend the cross-repository App quota.""" + calls = [] + + def fake_run_with_env(args, *, stdin=None, env=None): + calls.append((tuple(args), None if env is None else env.get("GH_TOKEN"))) + return '{"workflow_runs": []}' + + monkeypatch.setattr(sched, "run_with_env", fake_run_with_env) + monkeypatch.setenv("GH_TOKEN", "opencode-app-token") + monkeypatch.setenv("SCHEDULER_ACTIONS_TOKEN", "cross-repository-actions-token") + monkeypatch.setenv("SCHEDULER_DISPATCH_TOKEN", "central-runner-token") + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + + sched.active_workflow_runs("ContextualWisdomLab/.github", statuses=("queued",)) + sched.active_workflow_runs("owner/repo", statuses=("queued",)) + + assert [call[1] for call in calls] == [ + "central-runner-token", + "cross-repository-actions-token", + ] + + def test_missing_evidence_dispatch_uses_central_required_workflow_repository(monkeypatch): calls = [] head_sha = "a" * 40 From ebfff80463da7071af11971450ac73a4f2fb838a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 19:45:32 +0900 Subject: [PATCH 02/14] fix(scheduler): isolate central run cancellation quota --- CHANGELOG.md | 10 +++++----- docs/doctoring/fork-head-review-dispatch.md | 11 ++++++----- scripts/ci/pr_review_merge_scheduler.py | 10 +++++++++- tests/test_pr_review_merge_scheduler.py | 8 ++++++-- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1787c50283..3480131c76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,11 +47,11 @@ Semantic Versioning where the repository publishes a release. - Replaced nonexistent `job.workflow_repository` / `job.workflow_sha` / `job.workflow_ref` / `job.workflow_file_path` context references (actionlint: "property ... is not defined in object type") in `pr-review-fix-scheduler.yml`'s called-workflow source verification and `exact-artifact-sbom-attestation.yml`'s trusted-verifier checkout. Both always failed closed on the missing properties (ContextualWisdomLab/.github#1212) or, for the SBOM attestation checkout, silently resolved an empty repository/ref instead of the pinned trusted source (downstream `gh attestation verify --signer-repo`/`--signer-workflow`, using the separately hardcoded `SIGNER_REPOSITORY` constant rather than any workflow_ref, still failed closed on the resulting empty signer identity). `github.workflow_ref`/`github.workflow_sha` are real, documented properties, but for a `workflow_call` target they reflect the top-level *calling* workflow, not the reusable workflow's own file — a prefix match against the reusable workflow's own path can never succeed. `exact-artifact-sbom-attestation.yml`'s checkout now uses `github.workflow_sha` (correct today: it has no callers yet); `pr-review-fix-scheduler.yml`'s identity check instead validates `github.repository`, since every current caller uses a local, same-repo `uses: ./...` where caller and callee share one commit and `github.workflow_sha` is still the right pin. Tracked follow-up for the SBOM attestation checkout once a real (potentially cross-repo) caller exists: ContextualWisdomLab/.github#1228. - Use the repository hosting each workflow run to select scheduler Actions - credentials: central required-workflow inventory uses the receiving - repository's job token, while target-repository inventory and mutations keep - the established explicit credential chain. An exhausted organization-wide - OpenCode App installation budget can no longer prevent either central or - cross-repository PRs from dispatching exact-head review. + credentials: central required-workflow inventory and stale-run cancellation + use the receiving repository's job token, while target-repository inventory + and mutations keep the established explicit credential chain. An exhausted + organization-wide OpenCode App installation budget can no longer prevent + either central or cross-repository PRs from dispatching exact-head review. - Kept independently valid root-level Python lock environments separate during trusted base coverage installation. A directory with more than two candidate locks no longer collapses unrelated OpenCode, security, and application diff --git a/docs/doctoring/fork-head-review-dispatch.md b/docs/doctoring/fork-head-review-dispatch.md index c854c2563b..e96ea09ef1 100644 --- a/docs/doctoring/fork-head-review-dispatch.md +++ b/docs/doctoring/fork-head-review-dispatch.md @@ -69,11 +69,12 @@ Targeted cross-repository run `32566396712` later exposed the remaining host boundary: while reviewing `contextual-orchestrator#820`, active OpenCode run discovery queried the central `.github` Actions inventory with the shared App token and exhausted that installation's quota before dispatch. Active-run -inventory now selects credentials by the repository hosting the run. Central -required-workflow runs use the receiving repository's job token; target-repo -run inventory and mutations retain the explicit cross-repository credential. -The regression exercises both hosts in one call sequence so a later refactor -cannot collapse them back onto one rate-limit bucket. +inventory and stale-run cancellation now select credentials by the repository +hosting the run. Central required-workflow runs use the receiving repository's +job token; target-repo run inventory and mutations retain the explicit +cross-repository credential. The regression exercises discovery and +cancellation on both hosts so a later refactor cannot collapse them back onto +one rate-limit bucket. ## APA 7th references diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 8734d1de72..2d212423ca 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2100,11 +2100,19 @@ def force_cancel_workflow_runs(repo: str, run_ids: Sequence[str]) -> dict[str, s """Force-cancel workflow runs without blocking current-head decisions.""" if not run_ids: return {} + central_repo = ( + os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "" + ).strip() + run_command = ( + run_github_dispatch + if central_repo and repo == central_repo + else run_github_actions + ) def cancel_one(run_id: str) -> tuple[str, str | None]: """Return one run id and its bounded GitHub cancellation error, if any.""" try: - run_github_actions( + run_command( [ "gh", "api", diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 0c65b001eb..e72bea6eea 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1967,10 +1967,10 @@ def fake_run_with_env(args, *, stdin=None, env=None): ] -def test_active_workflow_runs_use_central_runner_token_for_central_dispatch( +def test_central_workflow_runs_use_central_runner_token_for_central_dispatch( monkeypatch, ): - """Central run discovery must not spend the cross-repository App quota.""" + """Central run discovery and cancellation must not spend the App quota.""" calls = [] def fake_run_with_env(args, *, stdin=None, env=None): @@ -1987,10 +1987,14 @@ def fake_run_with_env(args, *, stdin=None, env=None): ) sched.active_workflow_runs("ContextualWisdomLab/.github", statuses=("queued",)) + sched.force_cancel_workflow_runs("ContextualWisdomLab/.github", ["101"]) sched.active_workflow_runs("owner/repo", statuses=("queued",)) + sched.force_cancel_workflow_runs("owner/repo", ["202"]) assert [call[1] for call in calls] == [ "central-runner-token", + "central-runner-token", + "cross-repository-actions-token", "cross-repository-actions-token", ] From ca9c5b5aa47e92f7e8afb90cb70f7b0e164b2b02 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 20:23:08 +0900 Subject: [PATCH 03/14] fix(scheduler): deduplicate exact review runs --- CHANGELOG.md | 3 ++ docs/doctoring/fork-head-review-dispatch.md | 9 +++++ scripts/ci/pr_review_merge_scheduler.py | 41 ++++++++++----------- tests/test_pr_review_merge_scheduler.py | 13 ++++--- 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3480131c76..6429af23be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ Semantic Versioning where the repository publishes a release. and mutations keep the established explicit credential chain. An exhausted organization-wide OpenCode App installation budget can no longer prevent either central or cross-repository PRs from dispatching exact-head review. + Exact repository-dispatch titles are also matched before GitHub's `name` + field is treated as a workflow alias, preventing duplicate exact-head model + runs when that field contains the configured `run-name`. - Kept independently valid root-level Python lock environments separate during trusted base coverage installation. A directory with more than two candidate locks no longer collapses unrelated OpenCode, security, and application diff --git a/docs/doctoring/fork-head-review-dispatch.md b/docs/doctoring/fork-head-review-dispatch.md index e96ea09ef1..e71653bf67 100644 --- a/docs/doctoring/fork-head-review-dispatch.md +++ b/docs/doctoring/fork-head-review-dispatch.md @@ -76,6 +76,15 @@ cross-repository credential. The regression exercises discovery and cancellation on both hosts so a later refactor cannot collapse them back onto one rate-limit bucket. +Targeted scheduler run `32569094917` then exposed a second inventory boundary: +GitHub returned the configured `run-name` in the Actions run `name` field for an +already-running exact-head OpenCode dispatch. Filtering that field as a workflow +alias before checking the trusted exact dispatch title missed run `32569021159` +and created duplicate run `32569106868`, which was cancelled before model work. +Central dispatch inventory now validates the exact repository, PR, and head SHA +encoded in the dispatch title before applying the legacy workflow-name filter. +The regression covers both API shapes so only one exact-head model review runs. + ## APA 7th references GitHub, Inc. (n.d.-a). *REST API endpoints for pull requests*. GitHub Docs. diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 2d212423ca..c0550e4bde 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -601,6 +601,19 @@ def run_github_dispatch(args: Sequence[str], *, stdin: str | None = None) -> str return run_with_env(args, stdin=stdin, env=env) +def run_github_actions_for_repository( + repo: str, + args: Sequence[str], +) -> str: + """Run an Actions command with the credential scoped to its host repository.""" + central_repo = ( + os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "" + ).strip() + if central_repo and repo == central_repo: + return run_github_dispatch(args) + return run_github_actions(args) + + def split_repo(repo: str) -> tuple[str, str]: """Split an owner/name repository string into owner and repository name.""" try: @@ -1924,17 +1937,10 @@ def rerun_actions_job(repo: str, job_id: str, *, dry_run: bool, action: str) -> def active_workflow_runs(repo: str, statuses: Sequence[str] = ("queued", "in_progress")) -> list[dict[str, Any]]: """Return active workflow runs with the credential scoped to their host.""" runs: list[dict[str, Any]] = [] - central_repo = ( - os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "" - ).strip() - run_command = ( - run_github_dispatch - if central_repo and repo == central_repo - else run_github_actions - ) for status in statuses: payload = json.loads( - run_command( + run_github_actions_for_repository( + repo, [ "gh", "api", @@ -2017,9 +2023,6 @@ def active_review_run_refs( # must not suppress the central authenticated reviewer. for run_repo in (dispatch_repo,): for run_data in active_workflow_runs(run_repo, statuses): - run_name = str(run_data.get("name") or "") - if run_name != workflow and run_name not in workflow_aliases: - continue run_id = run_data.get("id") if not run_id: continue @@ -2039,6 +2042,9 @@ def active_review_run_refs( continue (current if dispatched_head == head else stale).append(run_ref) continue + run_name = str(run_data.get("name") or "") + if run_name != workflow and run_name not in workflow_aliases: + continue if centralized_dispatch: continue run_head = str(run_data.get("head_sha") or "").lower() @@ -2100,19 +2106,12 @@ def force_cancel_workflow_runs(repo: str, run_ids: Sequence[str]) -> dict[str, s """Force-cancel workflow runs without blocking current-head decisions.""" if not run_ids: return {} - central_repo = ( - os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "" - ).strip() - run_command = ( - run_github_dispatch - if central_repo and repo == central_repo - else run_github_actions - ) def cancel_one(run_id: str) -> tuple[str, str | None]: """Return one run id and its bounded GitHub cancellation error, if any.""" try: - run_command( + run_github_actions_for_repository( + repo, [ "gh", "api", diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index e72bea6eea..219d14e8e6 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -2264,10 +2264,11 @@ def fake_run(args, stdin=None): @pytest.mark.parametrize( - ("workflow_name", "run_title"), + ("workflow_name", "run_title", "configured_run_name"), [ - ("OpenCode Review Dispatch", "OpenCode Review Dispatch"), - ("Required OpenCode Review", "Required OpenCode Review"), + ("OpenCode Review Dispatch", "OpenCode Review Dispatch", False), + ("Required OpenCode Review", "Required OpenCode Review", False), + ("OpenCode Review Dispatch", "OpenCode Review Dispatch", True), ], ) def test_dispatch_opencode_review_deduplicates_current_head_repository_dispatch( @@ -2275,15 +2276,17 @@ def test_dispatch_opencode_review_deduplicates_current_head_repository_dispatch( capsys, workflow_name, run_title, + configured_run_name, ): calls = [] head_sha = "a" * 40 + display_title = f"{run_title} owner/repo#1@{head_sha}" current_dispatch = { "id": 9100, - "name": workflow_name, + "name": display_title if configured_run_name else workflow_name, "event": "repository_dispatch", "head_sha": "default-branch-sha", - "display_title": f"{run_title} owner/repo#1@{head_sha}", + "display_title": display_title, "pull_requests": [], } From 59c6df2f873c4f1ee2f5ad851dc10a7369cf4dcc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 21:28:28 +0900 Subject: [PATCH 04/14] fix(scheduler): bypass target Actions quota for central review --- CHANGELOG.md | 9 +++++---- docs/doctoring/fork-head-review-dispatch.md | 14 ++++++++++++++ scripts/ci/pr_review_merge_scheduler.py | 6 +++++- tests/test_pr_review_merge_scheduler.py | 21 +++++++++++++++++++++ 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6429af23be..0e222eedfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,10 +48,11 @@ Semantic Versioning where the repository publishes a release. - Replaced nonexistent `job.workflow_repository` / `job.workflow_sha` / `job.workflow_ref` / `job.workflow_file_path` context references (actionlint: "property ... is not defined in object type") in `pr-review-fix-scheduler.yml`'s called-workflow source verification and `exact-artifact-sbom-attestation.yml`'s trusted-verifier checkout. Both always failed closed on the missing properties (ContextualWisdomLab/.github#1212) or, for the SBOM attestation checkout, silently resolved an empty repository/ref instead of the pinned trusted source (downstream `gh attestation verify --signer-repo`/`--signer-workflow`, using the separately hardcoded `SIGNER_REPOSITORY` constant rather than any workflow_ref, still failed closed on the resulting empty signer identity). `github.workflow_ref`/`github.workflow_sha` are real, documented properties, but for a `workflow_call` target they reflect the top-level *calling* workflow, not the reusable workflow's own file — a prefix match against the reusable workflow's own path can never succeed. `exact-artifact-sbom-attestation.yml`'s checkout now uses `github.workflow_sha` (correct today: it has no callers yet); `pr-review-fix-scheduler.yml`'s identity check instead validates `github.repository`, since every current caller uses a local, same-repo `uses: ./...` where caller and callee share one commit and `github.workflow_sha` is still the right pin. Tracked follow-up for the SBOM attestation checkout once a real (potentially cross-repo) caller exists: ContextualWisdomLab/.github#1228. - Use the repository hosting each workflow run to select scheduler Actions credentials: central required-workflow inventory and stale-run cancellation - use the receiving repository's job token, while target-repository inventory - and mutations keep the established explicit credential chain. An exhausted - organization-wide OpenCode App installation budget can no longer prevent - either central or cross-repository PRs from dispatching exact-head review. + use the receiving repository's job token. Centrally hosted review dispatches + no longer enumerate or cancel non-authoritative target old-head CI before + dispatch; same-repository cleanup and explicit target mutations keep their + established credentials. An exhausted organization-wide OpenCode App + installation budget can no longer stop exact-head review on that cleanup read. Exact repository-dispatch titles are also matched before GitHub's `name` field is treated as a workflow alias, preventing duplicate exact-head model runs when that field contains the configured `run-name`. diff --git a/docs/doctoring/fork-head-review-dispatch.md b/docs/doctoring/fork-head-review-dispatch.md index e71653bf67..b73d20f016 100644 --- a/docs/doctoring/fork-head-review-dispatch.md +++ b/docs/doctoring/fork-head-review-dispatch.md @@ -85,6 +85,20 @@ Central dispatch inventory now validates the exact repository, PR, and head SHA encoded in the dispatch title before applying the legacy workflow-name filter. The regression covers both API shapes so only one exact-head model review runs. +Live retry `32572857921` exposed one remaining pre-dispatch quota consumer. Every +PR inspection unconditionally enumerated queued and running workflows in the +target repository to cancel old-head CI before it examined the centrally hosted +review run. The shared App installation was already rate-limited, so +`contextual-orchestrator#820` stopped on that non-authoritative cleanup read and +never reached exact-head review dispatch. When the required reviewer is hosted +centrally, target-repository old-head jobs do not supply current-head approval or +merge evidence and the central dispatch functions already deduplicate and cancel +their own stale review runs. Centralized inspections therefore skip only that +target old-head inventory/cancellation step. Same-repository schedulers retain it, +and all current-head checks, review identity, target reads needed for live PR +validation, and explicit target mutations remain fail-closed. This removes two +target Actions-list requests per inspected PR without widening any authority. + ## APA 7th references GitHub, Inc. (n.d.-a). *REST API endpoints for pull requests*. GitHub Docs. diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index c0550e4bde..85e8a25ca9 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2371,7 +2371,11 @@ def inspect_pr( if pr.get("isDraft"): return Decision(number, "skip", "draft PR") - cancel_stale_pr_runs(repo, pr, dry_run=dry_run) + # Central reviewers own their run lifecycle in the dispatch repository. + # Target old-head CI is non-authoritative, and enumerating it can exhaust + # the installation quota before the current-head review is dispatched. + if repository_dispatch_target(repo) == repo: + cancel_stale_pr_runs(repo, pr, dry_run=dry_run) if base_ref != base_branch: # Stacked/cascade PR (base is another feature branch). Org required # workflows are only injected for default-branch-target PRs, so these diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 219d14e8e6..4395f9ae99 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -3584,6 +3584,27 @@ def test_inspect_pr_cancels_stale_queued_runs_before_decision(monkeypatch): assert cancelled == [("owner/repo", 1, True)] +def test_central_dispatch_skips_non_authoritative_target_actions_inventory( + monkeypatch, +): + """Central review dispatch must not spend App quota on target old-head runs.""" + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + monkeypatch.setattr( + sched, + "cancel_stale_pr_runs", + lambda *args, **kwargs: pytest.fail( + "central dispatch must not enumerate target Actions runs" + ), + ) + + decision = inspect(make_pr(baseRefName="feature-base"), trigger_reviews=False) + + assert decision.action == "skip" + + def test_inspect_pr_blocks_auto_merge_for_approved_conflicts(monkeypatch): auto_merges = [] disables = [] From 7ad869e16ceb38dae91a7de807b80994db258bc6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 22:24:18 +0900 Subject: [PATCH 05/14] fix(scheduler): reject draft merge mutations --- CHANGELOG.md | 5 +++++ docs/doctoring/fork-head-review-dispatch.md | 21 +++++++++++++++++++++ scripts/ci/pr_review_merge_scheduler.py | 4 ++++ tests/test_pr_review_merge_scheduler.py | 15 +++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c845ced148..32416e6d40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,11 @@ Semantic Versioning where the repository publishes a release. Exact repository-dispatch titles are also matched before GitHub's `name` field is treated as a workflow alias, preventing duplicate exact-head model runs when that field contains the configured `run-name`. +- Refused draft pull requests again at both direct-merge and auto-merge mutation + boundaries, even though `inspect_pr` already skips drafts before any mutation. + This defense in depth makes a future caller unable to turn forged check or + review metadata into a draft merge and records Strix run `32573579932` finding + `vuln-0001` without adding a scanner allowlist or weakening required checks. - Kept independently valid root-level Python lock environments separate during trusted base coverage installation. A directory with more than two candidate locks no longer collapses unrelated OpenCode, security, and application diff --git a/docs/doctoring/fork-head-review-dispatch.md b/docs/doctoring/fork-head-review-dispatch.md index b73d20f016..133fd60fe0 100644 --- a/docs/doctoring/fork-head-review-dispatch.md +++ b/docs/doctoring/fork-head-review-dispatch.md @@ -99,6 +99,27 @@ and all current-head checks, review identity, target reads needed for live PR validation, and explicit target mutations remain fail-closed. This removes two target Actions-list requests per inspected PR without widening any authority. +## Draft merge defense in depth + +Exact-head Strix run `32573579932` reported `vuln-0001`, alleging that a draft +pull request could forge successful checks and reach merge without OpenCode +approval. The proposed proof of concept does not traverse the executable +control flow: `inspect_pr` returns `skip: draft PR` before stale-run cleanup, +review interpretation, auto-merge, or direct merge, and an arbitrary author's +review is not an exact-head OpenCode approval. The report also assumed a fork +pull-request token could create base-repository check runs and approvals, +contrary to the least-privilege fork boundary documented by GitHub (GitHub, +Inc., n.d.-b). + +The finding is retained as security evidence rather than broadly suppressed. +As defense in depth against a future caller bypassing `inspect_pr`, both guarded +merge mutation functions now reject `isDraft` before actor validation or any +GitHub call. The regression invokes both mutation boundaries with a valid head +SHA and asserts an exception plus zero outbound commands. The existing +top-level draft regression remains, and a new exact-head Strix run must clear +the changed code; no scanner severity, check requirement, workflow identity, or +finding allowlist changed. + ## APA 7th references GitHub, Inc. (n.d.-a). *REST API endpoints for pull requests*. GitHub Docs. diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 85e8a25ca9..62175f1162 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1583,6 +1583,8 @@ def run_head_guarded_merge( def enable_auto_merge(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: """Enable auto-merge for a PR at its current head using an allowed method.""" + if pr.get("isDraft"): + raise RuntimeError("enable-auto-merge refused for draft PR") number = str(pr["number"]) if dry_run: return @@ -1593,6 +1595,8 @@ def enable_auto_merge(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: def merge_pr(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: """Merge a current-head-approved PR immediately with a head guard.""" + if pr.get("isDraft"): + raise RuntimeError("direct-merge refused for draft PR") number = str(pr["number"]) if dry_run: return diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 4395f9ae99..8dd811fa51 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1742,6 +1742,21 @@ def fake_run(args, stdin=None): ] +def test_draft_pr_cannot_reach_merge_mutations(monkeypatch): + """Defense in depth rejects drafts at both guarded merge boundaries.""" + calls = [] + monkeypatch.setattr(sched, "run", lambda args: calls.append(args) or "") + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("GH_TOKEN", "workflow-token") + draft_pr = make_pr(isDraft=True, headRefOid="a" * 40) + + for mutation in (sched.enable_auto_merge, sched.merge_pr): + with pytest.raises(RuntimeError, match="draft PR"): + mutation("owner/repo", draft_pr, dry_run=False) + + assert calls == [] + + def test_last_push_approval_restamp_creates_same_tree_child(monkeypatch): calls = [] head_sha = "a" * 40 From 2ff95dbee2e9cbf6179c7c078001317ef5bc301c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 15:53:33 +0900 Subject: [PATCH 06/14] fix(scheduler): bind head mutations to selected token --- .../workflows/opencode-review-dispatch.yml | 1 + .../workflows/pr-review-merge-scheduler.yml | 2 + CHANGELOG.md | 7 +- docs/doctoring/fork-head-review-dispatch.md | 26 ++++++ scripts/ci/pr_review_merge_scheduler.py | 85 +++++++++++++------ tests/test_opencode_agent_contract.py | 2 + ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- tests/test_pr_review_merge_scheduler.py | 41 ++++++++- 8 files changed, 135 insertions(+), 31 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 3bc1ce6d38..5c776dfe26 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -8020,6 +8020,7 @@ jobs: SCHEDULER_ACTIONS_TOKEN: ${{ github.token }} SCHEDULER_READ_TOKEN: ${{ (github.event_name == 'pull_request_target' || needs.validate-pr-metadata.outputs.target_repository == github.repository) && github.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.opencode_app_token.outputs.token }} SCHEDULER_MUTATION_TOKEN_SOURCE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || steps.opencode_app_token.outputs.available == 'true' && 'opencode-app' || 'github-token' }} + SCHEDULER_WORKFLOW_TOKEN: ${{ github.token }} GH_REPOSITORY: ${{ needs.validate-pr-metadata.outputs.target_repository }} PR_BASE_REF: ${{ needs.validate-pr-metadata.outputs.base_ref }} PR_NUMBER: ${{ needs.validate-pr-metadata.outputs.pr_number }} diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 697038d1c0..0a97231840 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -494,6 +494,7 @@ jobs: SCHEDULER_DISPATCH_TOKEN: ${{ github.token }} SCHEDULER_READ_TOKEN: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && github.event.client_payload.target_repository != github.repository && (secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token) || github.token }} SCHEDULER_MUTATION_TOKEN_SOURCE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || steps.scheduler_app_token.outputs.available == 'true' && 'opencode-app' || 'github-token' }} + SCHEDULER_WORKFLOW_TOKEN: ${{ github.token }} SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY: ContextualWisdomLab/.github SCHEDULER_ALLOW_CROSS_REPO_REPOSITORY_DISPATCH: ${{ (secrets.PR_REVIEW_MERGE_TOKEN != '' || secrets.OPENCODE_APPROVE_TOKEN != '') && 'true' || 'false' }} run: | @@ -781,6 +782,7 @@ jobs: # "no cross-repository repository-dispatch credential". SCHEDULER_DISPATCH_TOKEN: ${{ github.token }} SCHEDULER_MUTATION_TOKEN_SOURCE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || steps.sweep_app_token.outputs.available == 'true' && 'opencode-app' || 'github-token' }} + SCHEDULER_WORKFLOW_TOKEN: ${{ github.token }} SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY: ContextualWisdomLab/.github SCHEDULER_ALLOW_CROSS_REPO_REPOSITORY_DISPATCH: ${{ (secrets.PR_REVIEW_MERGE_TOKEN != '' || secrets.OPENCODE_APPROVE_TOKEN != '') && 'true' || 'false' }} run: | diff --git a/CHANGELOG.md b/CHANGELOG.md index 32416e6d40..f41cfb74fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,7 +45,12 @@ Semantic Versioning where the repository publishes a release. ### Fixed -- Retried the Strix scan up to `STRIX_TRANSIENT_RETRY_PER_MODEL` times, same model, when the log shows the upstream strix-agent Caido sandbox bootstrap timing race (`loginAsGuest failed after N attempts` / `Failed to connect to 127.0.0.1 port `; tracked upstream as usestrix/strix#1036, #1037, #1056). A slow CI runner can exceed strix-agent's fixed 10-attempt sandbox-login budget before its local intercepting proxy is reachable, even though the penetration test itself never started and no vulnerability evidence was produced or lost; the Docker image is already cached from the failed attempt, so a same-model retry is cheap and typically clears the one-off boot race. Not wired into cross-model fallback, since switching LLM models cannot change local sandbox container boot timing. +- Bound head-mutation authorization to the actual selected `GH_TOKEN` as well + as its declared source, failing closed when it is missing or resolves to the + workflow `github.token`; case-fold repository host comparisons so casing + drift cannot select the wrong Actions credential or skip same-repository + stale-run cleanup. +- Retried the Strix scan up to `STRIX_TRANSIENT_RETRY_PER_MODEL` times, same model, when the log shows the upstream strix-agent Caido sandbox bootstrap timing race (`loginAsGuest failed after N attempts` / `Failed to connect to 127.0.0.1 port `; tracked upstream as usestrix/strix#1036, usestrix/strix#1037, usestrix/strix#1056). A slow CI runner can exceed strix-agent's fixed 10-attempt sandbox-login budget before its local intercepting proxy is reachable, even though the penetration test itself never started and no vulnerability evidence was produced or lost; the Docker image is already cached from the failed attempt, so a same-model retry is cheap and typically clears the one-off boot race. Not wired into cross-model fallback, since switching LLM models cannot change local sandbox container boot timing. - Replaced nonexistent `job.workflow_repository` / `job.workflow_sha` / `job.workflow_ref` / `job.workflow_file_path` context references (actionlint: "property ... is not defined in object type") in `pr-review-fix-scheduler.yml`'s called-workflow source verification and `exact-artifact-sbom-attestation.yml`'s trusted-verifier checkout. Both always failed closed on the missing properties (ContextualWisdomLab/.github#1212) or, for the SBOM attestation checkout, silently resolved an empty repository/ref instead of the pinned trusted source (downstream `gh attestation verify --signer-repo`/`--signer-workflow`, using the separately hardcoded `SIGNER_REPOSITORY` constant rather than any workflow_ref, still failed closed on the resulting empty signer identity). `github.workflow_ref`/`github.workflow_sha` are real, documented properties, but for a `workflow_call` target they reflect the top-level *calling* workflow, not the reusable workflow's own file — a prefix match against the reusable workflow's own path can never succeed. `exact-artifact-sbom-attestation.yml`'s checkout now uses `github.workflow_sha` (correct today: it has no callers yet); `pr-review-fix-scheduler.yml`'s identity check instead validates `github.repository`, since every current caller uses a local, same-repo `uses: ./...` where caller and callee share one commit and `github.workflow_sha` is still the right pin. Tracked follow-up for the SBOM attestation checkout once a real (potentially cross-repo) caller exists: ContextualWisdomLab/.github#1228. - Use the repository hosting each workflow run to select scheduler Actions credentials: central required-workflow inventory and stale-run cancellation diff --git a/docs/doctoring/fork-head-review-dispatch.md b/docs/doctoring/fork-head-review-dispatch.md index 133fd60fe0..ab48713d0a 100644 --- a/docs/doctoring/fork-head-review-dispatch.md +++ b/docs/doctoring/fork-head-review-dispatch.md @@ -99,6 +99,28 @@ and all current-head checks, review identity, target reads needed for live PR validation, and explicit target mutations remain fail-closed. This removes two target Actions-list requests per inspected PR without widening any authority. +Exact-head Strix [run 32579981586](https://github.com/ContextualWisdomLab/.github/actions/runs/32579981586) +then reported a HIGH mismatch between the +declared mutation-credential source and the token actually inherited by `gh`. +Its illustrative fallback helper was not present in the scheduler, and the +workflow expressions select `GH_TOKEN` and `SCHEDULER_MUTATION_TOKEN_SOURCE` +from the same precedence chain. The executable boundary nevertheless relied on +that expression-level coupling: a missing token or inconsistent GitHub App +`available` output could select the runner `github.token` while the Python +guard still trusted the stronger source label. + +Every scheduler mutation entrypoint now receives the runner token separately +as `SCHEDULER_WORKFLOW_TOKEN`. A head update is authorized only when the source +is allowlisted, the selected `GH_TOKEN` and comparison token are both present, +and the two actual token values differ. Neither value is logged. Tests cover an +empty selected token and a source-label/runner-token mismatch, while the offline +self-test uses distinct synthetic values. Repository-host identity comparisons +also use case-folded canonical names, so a case-only spelling difference cannot +move central Actions inventory onto a shared App credential or skip +same-repository stale-run cleanup. This is a zero-trust verification at the +mutation boundary rather than trust in an upstream environment label (Rose et +al., 2020). + ## Draft merge defense in depth Exact-head Strix run `32573579932` reported `vuln-0001`, alleging that a draft @@ -137,6 +159,10 @@ https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api GitHub, Inc. (n.d.-d). *GITHUB_TOKEN*. GitHub Docs. Retrieved August 22, 2026, from https://docs.github.com/en/actions/concepts/security/github_token +Rose, S., Borchert, O., Mitchell, S., & Connelly, S. (2020). *Zero trust +architecture* (NIST Special Publication 800-207). National Institute of +Standards and Technology. https://doi.org/10.6028/NIST.SP.800-207 + Souppaya, M., Scarfone, K., & Dodson, D. (2022). *Secure Software Development Framework (SSDF) version 1.1: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218). National diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 62175f1162..d3fc3340f0 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -230,8 +230,8 @@ def mutation_token_label() -> str: return labels.get(source, "workflow GH_TOKEN") -def head_mutation_credential_starts_workflows() -> bool: - """Return whether scheduler head mutations can start required workflow runs. +def head_mutation_credential_problem() -> str | None: + """Explain why the selected mutation credential cannot start workflow runs. GitHub never creates a new workflow run for an event produced with the workflow ``GITHUB_TOKEN``, so a PR head moved with that credential can never @@ -242,22 +242,41 @@ def head_mutation_credential_starts_workflows() -> bool: GitHub. (2025). *Automatic token authentication*. https://docs.github.com/actions/security-for-github-actions/security-guides/automatic-token-authentication """ - return mutation_token_source() in WORKFLOW_STARTING_MUTATION_SOURCES - - -def non_triggering_head_mutation_reason(action: str) -> str: - """Explain why a head mutation is withheld for a non-triggering credential.""" source = mutation_token_source() if source == "github-token": - credential_reason = ( - "the workflow GITHUB_TOKEN, whose head mutations never start new workflow runs" + return "the workflow GITHUB_TOKEN, whose head mutations never start new workflow runs" + if source not in WORKFLOW_STARTING_MUTATION_SOURCES: + return f"{mutation_token_label()} is not allowlisted as workflow-starting" + + selected_token = (os.environ.get("GH_TOKEN") or "").strip() + workflow_token = (os.environ.get("SCHEDULER_WORKFLOW_TOKEN") or "").strip() + if not selected_token: + return f"{mutation_token_label()} is missing and therefore not proven workflow-starting" + if not workflow_token: + return ( + "workflow GITHUB_TOKEN comparison evidence is missing, so the selected mutation " + "credential is not proven workflow-starting" ) - else: - credential_reason = ( - f"the {mutation_token_label()}, which is not allowlisted as workflow-starting" + if selected_token == workflow_token: + return ( + f"{mutation_token_label()} resolved to the workflow GITHUB_TOKEN, whose head " + "mutations never start new workflow runs" ) + return None + + +def head_mutation_credential_starts_workflows() -> bool: + """Return whether the actual scheduler mutation token can start workflow runs.""" + return head_mutation_credential_problem() is None + + +def non_triggering_head_mutation_reason(action: str) -> str: + """Explain why a head mutation is withheld for a non-triggering credential.""" + credential_reason = head_mutation_credential_problem() + if credential_reason is None: + credential_reason = "the selected mutation credential is not proven workflow-starting" return ( - f"{action} withheld because the scheduler mutation credential is {credential_reason}, " + f"{action} withheld because {credential_reason}, " "so the moved head would stay permanently " "BLOCKED without current-head required checks; configure PR_REVIEW_MERGE_TOKEN, " "OPENCODE_APPROVE_TOKEN, or the OpenCode app token for the scheduler job" @@ -272,13 +291,11 @@ def require_workflow_starting_mutation_credential(action: str) -> None: def head_mutation_credential_guidance_text() -> tuple[str, str]: """Return operator-facing summary and limit text for a withheld head mutation.""" - if mutation_token_source() == "github-token": - return ( - "The scheduler withheld a head mutation because the workflow GITHUB_TOKEN cannot start the required current-head workflow runs.", - "Moving the head with the workflow GITHUB_TOKEN would leave the PR permanently BLOCKED, so the scheduler waits instead.", - ) + problem = head_mutation_credential_problem() + if problem is None: + problem = "the selected mutation credential is not proven workflow-starting" return ( - f"The scheduler withheld a head mutation because {mutation_token_label()} is not allowlisted as workflow-starting.", + f"The scheduler withheld a head mutation because {problem}.", "Moving the head is unsafe until the scheduler can prove that the selected credential starts the required current-head workflow runs.", ) @@ -609,7 +626,7 @@ def run_github_actions_for_repository( central_repo = ( os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "" ).strip() - if central_repo and repo == central_repo: + if central_repo and repo.casefold() == central_repo.casefold(): return run_github_dispatch(args) return run_github_actions(args) @@ -2378,7 +2395,7 @@ def inspect_pr( # Central reviewers own their run lifecycle in the dispatch repository. # Target old-head CI is non-authoritative, and enumerating it can exhaust # the installation quota before the current-head review is dispatched. - if repository_dispatch_target(repo) == repo: + if repository_dispatch_target(repo).casefold() == repo.casefold(): cancel_stale_pr_runs(repo, pr, dry_run=dry_run) if base_ref != base_branch: # Stacked/cascade PR (base is another feature branch). Org required @@ -3123,6 +3140,8 @@ def parse_non_triggering_head_mutation_reason(reason: str) -> bool: return ( "whose head mutations never start new workflow runs" in reason or "which is not allowlisted as workflow-starting" in reason + or "is not allowlisted as workflow-starting" in reason + or "not proven workflow-starting" in reason ) @@ -3320,16 +3339,28 @@ def summarize_action_error(exc: RuntimeError) -> str: @contextlib.contextmanager def declared_mutation_token_source(source: str) -> Iterator[None]: - """Declare a scheduler mutation credential source for the enclosed block.""" - previous = os.environ.get("SCHEDULER_MUTATION_TOKEN_SOURCE") + """Declare a coherent synthetic mutation credential for offline self-tests.""" + keys = ( + "SCHEDULER_MUTATION_TOKEN_SOURCE", + "GH_TOKEN", + "SCHEDULER_WORKFLOW_TOKEN", + ) + previous = {key: os.environ.get(key) for key in keys} os.environ["SCHEDULER_MUTATION_TOKEN_SOURCE"] = source + os.environ["SCHEDULER_WORKFLOW_TOKEN"] = "self-test-workflow-token" + os.environ["GH_TOKEN"] = ( + "self-test-workflow-token" + if source == "github-token" + else "self-test-selected-mutation-token" + ) try: yield finally: - if previous is None: - os.environ.pop("SCHEDULER_MUTATION_TOKEN_SOURCE", None) - else: - os.environ["SCHEDULER_MUTATION_TOKEN_SOURCE"] = previous + for key, value in previous.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value def self_test() -> None: diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index aaea3b0eb3..0a22724e2e 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -1996,6 +1996,7 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): in workflow ) assert "SCHEDULER_MUTATION_TOKEN_SOURCE" in workflow + assert workflow.count("SCHEDULER_WORKFLOW_TOKEN: ${{ github.token }}") == 2 assert 'default: "1"' in workflow assert 'review_dispatch_limit="-1"' in workflow assert "branch_update_limit:" in workflow @@ -2090,6 +2091,7 @@ def test_opencode_runs_merge_scheduler_after_review_without_repo_local_dispatch( "'OPENCODE_APPROVE_TOKEN' || steps.opencode_app_token.outputs.available == 'true' && " "'opencode-app' || 'github-token' }}" ) in workflow + assert "SCHEDULER_WORKFLOW_TOKEN: ${{ github.token }}" in workflow assert "--no-trigger-reviews" in workflow assert "--enable-auto-merge" in workflow assert "--no-update-branches" in workflow diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index d2d87b9e38..970a3a470e 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "3bc1ce6d385bce569e7a7ba037f149a8f18039d4" +REVIEW_DISPATCH_BLOB_SHA = "5c776dfe26629bdc08db110ea1ade9bd367a6a3a" def _workflow_text(path: Path) -> str: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 8dd811fa51..ac9bd8a0cb 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -33,6 +33,8 @@ def workflow_starting_mutation_credential(monkeypatch): workflow-starting credential exactly like the scheduler workflow does. """ monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "PR_REVIEW_MERGE_TOKEN") + monkeypatch.setenv("GH_TOKEN", "selected-mutation-token") + monkeypatch.setenv("SCHEDULER_WORKFLOW_TOKEN", "workflow-runner-token") def fake_github_token(prefix, body): @@ -1815,16 +1817,26 @@ def test_head_mutations_refuse_the_workflow_github_token(monkeypatch): def test_declared_mutation_token_source_restores_the_previous_environment(monkeypatch): - """The declaration helper restores both a set and an unset prior value.""" + """The declaration helper restores source and token evidence after self-tests.""" monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "opencode-app") + monkeypatch.setenv("GH_TOKEN", "prior-selected-token") + monkeypatch.setenv("SCHEDULER_WORKFLOW_TOKEN", "prior-workflow-token") with sched.declared_mutation_token_source("github-token"): assert sched.mutation_token_source() == "github-token" + assert not sched.head_mutation_credential_starts_workflows() assert sched.mutation_token_source() == "opencode-app" + assert os.environ["GH_TOKEN"] == "prior-selected-token" + assert os.environ["SCHEDULER_WORKFLOW_TOKEN"] == "prior-workflow-token" monkeypatch.delenv("SCHEDULER_MUTATION_TOKEN_SOURCE", raising=False) + monkeypatch.delenv("GH_TOKEN", raising=False) + monkeypatch.delenv("SCHEDULER_WORKFLOW_TOKEN", raising=False) with sched.declared_mutation_token_source("PR_REVIEW_MERGE_TOKEN"): assert sched.mutation_token_source() == "PR_REVIEW_MERGE_TOKEN" + assert sched.head_mutation_credential_starts_workflows() assert "SCHEDULER_MUTATION_TOKEN_SOURCE" not in os.environ + assert "GH_TOKEN" not in os.environ + assert "SCHEDULER_WORKFLOW_TOKEN" not in os.environ def test_workflow_starting_credentials_allow_head_mutations(monkeypatch): @@ -1835,6 +1847,30 @@ def test_workflow_starting_credentials_allow_head_mutations(monkeypatch): sched.require_workflow_starting_mutation_credential("update-branch") +@pytest.mark.parametrize( + ("selected_token", "workflow_token", "message"), + ( + ("", "workflow-runner-token", "is missing"), + ("selected-mutation-token", "", "comparison evidence is missing"), + ("workflow-runner-token", "workflow-runner-token", "resolved to"), + ), +) +def test_declared_workflow_starting_source_cannot_mask_runner_token_fallback( + monkeypatch, + selected_token, + workflow_token, + message, +): + """A missing credential that resolves to github.token cannot move a PR head.""" + monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "PR_REVIEW_MERGE_TOKEN") + monkeypatch.setenv("GH_TOKEN", selected_token) + monkeypatch.setenv("SCHEDULER_WORKFLOW_TOKEN", workflow_token) + + assert not sched.head_mutation_credential_starts_workflows() + with pytest.raises(RuntimeError, match=message): + sched.require_workflow_starting_mutation_credential("update-branch") + + def test_unknown_mutation_credential_source_is_fail_closed(monkeypatch): """An unrecognized credential source cannot authorize a head mutation.""" monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "unrecognized-token") @@ -1998,7 +2034,7 @@ def fake_run_with_env(args, *, stdin=None, env=None): monkeypatch.setenv("SCHEDULER_DISPATCH_TOKEN", "central-runner-token") monkeypatch.setenv( "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", - "ContextualWisdomLab/.github", + "contextualwisdomlab/.GITHUB", ) sched.active_workflow_runs("ContextualWisdomLab/.github", statuses=("queued",)) @@ -3586,6 +3622,7 @@ def test_workflow_run_filters_skip_mismatched_workflow_and_current_head_other_pr def test_inspect_pr_cancels_stale_queued_runs_before_decision(monkeypatch): + monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", "OWNER/REPO") cancelled = [] monkeypatch.setattr( sched, From 131f494922c7ac9336195d1facebb705e5ed75cb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 04:01:15 -0700 Subject: [PATCH 07/14] fix(ci): lint modern Actions schemas safely (#1247) * fix(ci): lint modern Actions schemas safely * fix(ci): preserve multiline workflow diagnostics * test(ci): bound Ruby runtime coverage * fix(ci): preserve deterministic workflow evidence --- .../exact-artifact-sbom-attestation.yml | 25 +- .../workflows/opencode-review-dispatch.yml | 61 +++- .github/workflows/pr-review-autofix.yml | 5 +- CHANGELOG.md | 6 + ...actionlint-modern-schema-and-shellcheck.md | 66 ++++ scripts/ci/lint_github_workflows.rb | 202 ++++++++++++ ...xact_artifact_sbom_attestation_contract.py | 3 + tests/test_lint_github_workflows.py | 307 ++++++++++++++++++ ...encode_rust_coverage_toolchain_contract.py | 13 + ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 10 files changed, 671 insertions(+), 19 deletions(-) create mode 100644 docs/doctoring/actionlint-modern-schema-and-shellcheck.md create mode 100644 scripts/ci/lint_github_workflows.rb create mode 100644 tests/test_lint_github_workflows.py diff --git a/.github/workflows/exact-artifact-sbom-attestation.yml b/.github/workflows/exact-artifact-sbom-attestation.yml index ea9aa4ed08..13edb2ca66 100644 --- a/.github/workflows/exact-artifact-sbom-attestation.yml +++ b/.github/workflows/exact-artifact-sbom-attestation.yml @@ -313,13 +313,13 @@ jobs: EOF { printf '\n## Exact signed identity\n\n' - printf -- '- Source repository: `%s`\n' "$SOURCE_REPOSITORY" - printf -- '- Source SHA: `%s`\n' "$SOURCE_SHA" - printf -- '- Signer repository: `%s`\n' "$SIGNER_REPOSITORY" - printf -- '- Signer workflow: `%s`\n' "$signer_workflow" - printf -- '- Predicate type: `%s`\n' "$PREDICATE_TYPE" - printf -- '- Wheel: `%s`\n' "$WHEEL_FILENAME" - printf -- '- Source distribution: `%s`\n' "$SDIST_FILENAME" + printf -- "- Source repository: \`%s\`\n" "$SOURCE_REPOSITORY" + printf -- "- Source SHA: \`%s\`\n" "$SOURCE_SHA" + printf -- "- Signer repository: \`%s\`\n" "$SIGNER_REPOSITORY" + printf -- "- Signer workflow: \`%s\`\n" "$signer_workflow" + printf -- "- Predicate type: \`%s\`\n" "$PREDICATE_TYPE" + printf -- "- Wheel: \`%s\`\n" "$WHEEL_FILENAME" + printf -- "- Source distribution: \`%s\`\n" "$SDIST_FILENAME" cat <> offline-attestation-evidence/README.md ( cd offline-attestation-evidence + evidence_file_list="$(mktemp "${RUNNER_TEMP}/offline-attestation-files.XXXXXX")" LC_ALL=C find . -maxdepth 1 -type f ! -name SHA256SUMS -printf '%f\n' \ - | LC_ALL=C sort \ - | while IFS= read -r evidence_file; do - sha256sum "$evidence_file" - done > SHA256SUMS + | LC_ALL=C sort > "$evidence_file_list" + mapfile -t evidence_files < "$evidence_file_list" + rm -f "$evidence_file_list" + for evidence_file in "${evidence_files[@]}"; do + sha256sum "$evidence_file" + done > SHA256SUMS ) chmod 0444 \ offline-attestation-evidence/README.md \ diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 3ba7c77db5..921c4a9b25 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -900,9 +900,11 @@ jobs: } append_command() { - printf '$ ' >>"$summary_file" - printf '%q ' "$@" >>"$summary_file" - printf '\n' >>"$summary_file" + { + printf '$ ' + printf '%q ' "$@" + printf '\n' + } >>"$summary_file" } emit_captured_log() { @@ -1175,6 +1177,8 @@ jobs: --command-json "$configured_command_json" done <<<"$configured_commands_json" else + # The child shell expands its own cwd and PYTHONPATH. + # shellcheck disable=SC2016 run_and_capture "Python coverage with missing-line report (${project_dir})" \ bash -c 'cd "$1" && PYTHONPATH="$([ -d src ] && printf src:. || printf .)" python3 -m coverage run -m pytest tests && python3 -m coverage report --show-missing' bash "$project_dir" fi @@ -1182,6 +1186,8 @@ jobs: if [ "$measured_projects" -eq 0 ]; then if has_tracked_files '*.py'; then + # The child shell resolves the checked-out source layout. + # shellcheck disable=SC2016 run_and_capture "Python coverage with missing-line report" \ bash -c 'PYTHONPATH="$([ -d src ] && printf src:. || printf .)" python3 -m coverage run -m pytest && python3 -m coverage report --show-missing' elif python3 -I -c 'import pytest_cov' >/dev/null 2>&1; then @@ -1315,6 +1321,8 @@ jobs: while IFS= read -r project_dir; do if [ -f "${project_dir}/tests/test_docstrings.py" ]; then measured_projects=1 + # The child shell expands its own positional cwd. + # shellcheck disable=SC2016 run_and_capture "Python docstring coverage (${project_dir})" \ bash -c 'cd "$1" && PYTHONPATH="$([ -d src ] && printf src:. || printf .)" python3 -m pytest tests/test_docstrings.py' bash "$project_dir" fi @@ -1620,6 +1628,8 @@ jobs: if [ -n "$package_name" ] && jq -e '.workspaces // empty' package.json >/dev/null 2>&1; then run_and_capture "Tauri frontendDist build (${package_dir})" npm run build --workspace "$package_name" else + # The child shell expands its own positional cwd. + # shellcheck disable=SC2016 run_and_capture "Tauri frontendDist build (${package_dir})" bash -c 'cd "$1" && npm run build' bash "$package_dir" fi ;; @@ -1627,6 +1637,8 @@ jobs: if [ -n "$package_name" ] && [ -f pnpm-workspace.yaml ]; then run_and_capture "Tauri frontendDist build (${package_dir})" corepack pnpm --filter "$package_name" run build else + # The child shell expands its own positional cwd. + # shellcheck disable=SC2016 run_and_capture "Tauri frontendDist build (${package_dir})" bash -c 'cd "$1" && corepack pnpm run build' bash "$package_dir" fi ;; @@ -1634,6 +1646,8 @@ jobs: if [ -n "$package_name" ] && jq -e '.workspaces // empty' package.json >/dev/null 2>&1; then run_and_capture "Tauri frontendDist build (${package_dir})" yarn workspace "$package_name" build else + # The child shell expands its own positional cwd. + # shellcheck disable=SC2016 run_and_capture "Tauri frontendDist build (${package_dir})" bash -c 'cd "$1" && yarn build' bash "$package_dir" fi ;; @@ -1750,8 +1764,14 @@ jobs: # coverage command still runs and reports any uncovered GPU lines # exactly as before, so Rust repositories without GPU code are # unaffected and no gate is weakened. - if ls /usr/share/vulkan/icd.d/lvp_icd*.json >/dev/null 2>&1; then - lvp_icd="$(ls /usr/share/vulkan/icd.d/lvp_icd*.json | head -n1)" + lvp_icd="" + for candidate in /usr/share/vulkan/icd.d/lvp_icd*.json; do + if [ -f "$candidate" ]; then + lvp_icd="$candidate" + break + fi + done + if [ -n "$lvp_icd" ]; then export VK_ICD_FILENAMES="$lvp_icd" export VK_DRIVER_FILES="$lvp_icd" export WGPU_BACKEND=vulkan @@ -2875,12 +2895,17 @@ jobs: language_signal="Match changed prose" fi + # Markdown backticks are literal; the format argument is intentional. + # shellcheck disable=SC2016 printf -- '- Preferred review language: `%s`\n' "$language_signal" printf -- '- Rule: write human-readable review prose in the preferred language; keep file paths, identifiers, logs, quoted source, error text, and protocol literals unchanged.\n' + # shellcheck disable=SC2016 printf -- '- PR title: `%s`\n' "$(printf '%s' "$title" | tr '\r\n`' ' ' | cut -c 1-240)" if [ -n "$body" ]; then + # shellcheck disable=SC2016 printf -- '- PR body excerpt: `%s`\n' "$(printf '%s' "$body" | tr '\r\n`' ' ' | cut -c 1-360)" else + # shellcheck disable=SC2016 printf -- '- PR body excerpt: `[empty]`\n' fi } @@ -3160,6 +3185,8 @@ jobs: shift if ! git -C "$OPENCODE_SOURCE_WORKDIR" diff "$@"; then + # Markdown backticks are literal; the format arguments are intentional. + # shellcheck disable=SC2016 printf 'Unable to collect %s from `%s` to `%s`; continue review from available changed-file evidence and direct file inspection.\n' "$description" "$PR_MERGE_BASE" "$PR_HEAD_SHA" fi } @@ -3170,12 +3197,14 @@ jobs: printf -- "- Base SHA: \`%s\`\n" "$PR_BASE_SHA" printf -- "- Head SHA: \`%s\`\n\n" "$PR_HEAD_SHA" if ! PR_MERGE_BASE="$(git -C "$OPENCODE_SOURCE_WORKDIR" merge-base "$PR_BASE_SHA" "$PR_HEAD_SHA")"; then + # shellcheck disable=SC2016 printf 'Merge-base discovery failed for `%s` and `%s`; falling back to base SHA for bounded diff evidence.\n\n' "$PR_BASE_SHA" "$PR_HEAD_SHA" PR_MERGE_BASE="$PR_BASE_SHA" fi printf -- "- Merge base SHA: \`%s\`\n\n" "$PR_MERGE_BASE" printf '## Current-head authority order\n\n' printf 'Treat current-head sections in this file as authoritative for this run: Other unresolved review thread evidence, Failed GitHub Check evidence, Coverage execution evidence, Changed files, and Focused changed hunks.\n' + # shellcheck disable=SC2016 printf 'All PR reviews and comments evidence is historical context only and may contain stale bot conclusions. Do not infer active failed checks, unresolved threads, or missing changed files from those comments unless current-head evidence corroborates the same claim for Head SHA `%s`.\n\n' "$PR_HEAD_SHA" if ! git -C "$OPENCODE_SOURCE_WORKDIR" diff --name-only --find-renames "$PR_MERGE_BASE" "$PR_HEAD_SHA" | awk 'NF > 0 && $0 !~ /^\// && $0 !~ /(^|\/)\.\.($|\/)/ { print }' >"$OPENCODE_CHANGED_FILES_FILE"; then @@ -4811,6 +4840,8 @@ jobs: "$@" } + # jq expands its own variables inside this literal program. + # shellcheck disable=SC2016 self_check_filter=' def self_check: (.name // "") as $n @@ -5473,6 +5504,8 @@ jobs: if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then { printf '## OpenCode %s review body\n\n' "$event" + # Markdown backticks are literal; the format argument is intentional. + # shellcheck disable=SC2016 printf -- '- Head SHA: `%s`\n' "$HEAD_SHA" printf -- '- Workflow run: %s\n' "$RUN_ID" printf -- '- Workflow attempt: %s\n\n' "$RUN_ATTEMPT" @@ -5819,6 +5852,8 @@ jobs: printf '## Summary\n\n' printf '%s\n\n' "$summary" printf '## Adversarial validation\n\n' + # Markdown fences are literal; the format argument is intentional. + # shellcheck disable=SC2016 printf '```json\n%s\n```\n\n' "$adversarial_evidence" printf -- '- Result: REQUEST_CHANGES\n' printf -- '- Reason: %s\n\n' "$reason" @@ -6616,6 +6651,8 @@ jobs: case "$mode" in failed) + # jq expands its own variables inside this literal program. + # shellcheck disable=SC2016 jq_filter=' [.[].check_runs[]?] | sort_by((.started_at // .completed_at // .created_at // ""), (.id // 0)) @@ -6632,6 +6669,8 @@ jobs: ' ;; pending) + # jq expands its own variables inside this literal program. + # shellcheck disable=SC2016 jq_filter=' [.[].check_runs[]?] | sort_by((.started_at // .completed_at // .created_at // ""), (.id // 0)) @@ -6696,6 +6735,8 @@ jobs: local owner="${GH_REPOSITORY%%/*}" local name="${GH_REPOSITORY#*/}" + # GraphQL variables are expanded by GitHub, not Bash. + # shellcheck disable=SC2016 timeout "$(check_lookup_api_timeout_seconds)s" gh api graphql \ -f owner="$owner" \ -f name="$name" \ @@ -6849,6 +6890,8 @@ jobs: commit_check_runs_file="$(mktemp)" filtered_rollup_file="$(mktemp)" successful_check_names_file="$(mktemp)" + # GraphQL variables are expanded by GitHub, not Bash. + # shellcheck disable=SC2016 if ! pr_node_id="$(timeout "$(check_lookup_api_timeout_seconds)s" gh api graphql \ -f owner="$owner" \ -f name="$name" \ @@ -7292,6 +7335,8 @@ jobs: head_ref="$(printf '%s\n' "$pr_json" | jq -r '.headRefName // empty')" [ -n "$head_ref" ] || return 1 lookup_error_file="$(mktemp)" + # jq expands its own variables inside this literal program. + # shellcheck disable=SC2016 if ! GH_TOKEN="$scan_token" timeout "$(check_lookup_api_timeout_seconds)s" \ gh api -X GET "repos/${GH_REPOSITORY}/code-scanning/alerts" \ -f "ref=refs/heads/${head_ref}" \ @@ -7359,6 +7404,8 @@ jobs: printf 'OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.\n\n' printf '## Findings\n\n' printf '### 1. HIGH Current-head GitHub Checks - Fix failed required checks before approval\n' + # Markdown backticks are literal; the format argument is intentional. + # shellcheck disable=SC2016 printf -- '- Problem: Failed same-head checks remain for `%s`.\n' "$HEAD_SHA" printf -- '- Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.\n' printf -- '- Fix: Read and fix the failed check logs below, then rerun the current-head checks.\n' @@ -7415,10 +7462,14 @@ jobs: if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then { printf '## OpenCode required check satisfied by existing same-head approval\n\n' + # Markdown backticks are literal in these format strings. + # shellcheck disable=SC2016 printf -- '- Result: `EXISTING_CURRENT_HEAD_APPROVAL`\n' + # shellcheck disable=SC2016 printf -- '- Head SHA: `%s`\n' "$HEAD_SHA" printf -- '- Workflow run: %s\n' "$RUN_ID" printf -- '- Workflow attempt: %s\n' "$RUN_ATTEMPT" + # shellcheck disable=SC2016 printf -- '- Model-pool outcome: `%s`\n' "${OPENCODE_MODEL_POOL_OUTCOME:-unknown}" printf -- '- Reason: a prior real-model OpenCode APPROVED review with passed structured adversarial probes already targets this exact head, and the fallback rechecked coverage, peer checks, code-scanning alerts, and unresolved review threads before accepting it.\n' printf -- '- Review state: unchanged; no duplicate APPROVE review was posted from model-output-unavailable evidence.\n\n' diff --git a/.github/workflows/pr-review-autofix.yml b/.github/workflows/pr-review-autofix.yml index 7863577224..49f4b34aa4 100644 --- a/.github/workflows/pr-review-autofix.yml +++ b/.github/workflows/pr-review-autofix.yml @@ -509,8 +509,9 @@ jobs: if [ "${#changed_python_files[@]}" -gt 0 ]; then python3 -m py_compile "${changed_python_files[@]}" fi - if [ "${#changed_workflows[@]}" -gt 0 ] && command -v actionlint >/dev/null 2>&1; then - actionlint "${changed_workflows[@]}" + if [ "${#changed_workflows[@]}" -gt 0 ]; then + ruby "$GITHUB_WORKSPACE/trusted-autofix-source/scripts/ci/lint_github_workflows.rb" \ + "${changed_workflows[@]}" fi - name: Commit and push autofix diff --git a/CHANGELOG.md b/CHANGELOG.md index 55a3f0bcdb..196b713054 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,12 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Separate actionlint schema/expression/Pyflakes validation from file-based + ShellCheck execution so workflow shell blocks larger than 64 KiB cannot + deadlock the write-capable autofix verifier, preserve actionlint's shell and + expression semantics, and narrowly accept GitHub's native + `concurrency.queue: max` while rejecting every other queue value until + upstream actionlint schema support is released. - Bound head-mutation authorization to the actual selected `GH_TOKEN` as well as its declared source, failing closed when it is missing or resolves to the workflow `github.token`; case-fold repository host comparisons so casing diff --git a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md new file mode 100644 index 0000000000..af5b255f18 --- /dev/null +++ b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md @@ -0,0 +1,66 @@ +# Actionlint modern-schema and large-shell compatibility + +Decision date: **2026-08-22** + +## Incident + +The write-capable PR autofix worker validates every workflow it changes with +`actionlint`. Two upstream gaps can make that fail or stall even when GitHub +accepts the workflow. + +1. GitHub Actions supports `queue: max` for concurrency groups, while released + actionlint 1.7.12 still reports that key as invalid. Upstream pull request + 654 tracks schema support. +2. Actionlint can deadlock while sending a workflow `run` block larger than a + pipe buffer to its ShellCheck subprocess. Upstream issue 712 reproduces the + boundary at 64 KiB. The central OpenCode review workflow contains larger + trusted shell blocks, so an autofix touching it can wait indefinitely. + +These are linter transport/schema gaps, not reasons to remove workflow schema +validation or shell analysis. + +## Decision + +Keep actionlint as the schema, expression, and Pyflakes validator, but disable +only its ShellCheck subprocess integration with `-shellcheck=`. The trusted +`lint_github_workflows.rb` boundary uses Ruby's standard-library Psych parser to +read the same YAML scalar values, reproduces actionlint 1.7.12's workflow/job/ +runner/step shell precedence, expression normalization, implicit shell setup, +and narrow rule exclusions, and invokes the installed ShellCheck against unique +regular temporary files. It parses ShellCheck JSON, restores the workflow job +and step identity in every diagnostic, preserves findings as a failing status, +and fails closed on malformed output or a missing executable. + +The autofix worker ignores only actionlint's exact released-schema diagnostic +for the concurrency `queue` key. Before linting, it rejects every changed +workflow whose `queue` value is not exactly `max`; therefore the compatibility +exception cannot admit an invented queue mode. + +This is a temporary compatibility boundary. Remove the queue diagnostic +exception after an actionlint release containing pull request 654 is pinned. +Remove the stdin spool only after issue 712 is fixed and a greater-than-64-KiB +regression passes directly through the pinned actionlint/ShellCheck pair. + +## Verification + +- A greater-than-64-KiB synthetic shell program reaches the delegated + ShellCheck executable through a regular file, without content loss. +- Bash, sh, Windows/PowerShell, Python, workflow defaults, and GitHub expression + normalization retain actionlint's effective-shell behavior. +- ShellCheck findings, malformed result JSON, actionlint failures, and invalid + concurrency queue values all fail closed with actionable workflow context. +- The offline Python-only coverage sandbox records the Ruby subprocess + contracts as unavailable instead of failing with `FileNotFoundError`; the + hosted quality job, whose runner includes Ruby, executes those contracts and + the real all-workflow lint command. + +## References + +GitHub. (2026, May 7). *GitHub Actions concurrency groups now allow larger +queues*. https://github.blog/changelog/2026-05-07-github-actions-concurrency-groups-now-allow-larger-queues/ + +Murai, R. (2025). *Support queue: max in concurrency* [Pull request #654]. +GitHub. https://github.com/rhysd/actionlint/pull/654 + +Murai, R. (2026). *Shellcheck integration deadlocks for run blocks greater than +64 KiB* [Issue #712]. GitHub. https://github.com/rhysd/actionlint/issues/712 diff --git a/scripts/ci/lint_github_workflows.rb b/scripts/ci/lint_github_workflows.rb new file mode 100644 index 0000000000..620c419980 --- /dev/null +++ b/scripts/ci/lint_github_workflows.rb @@ -0,0 +1,202 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Run actionlint without its oversized-stdin ShellCheck transport, then apply +# the same ShellCheck policy directly to regular temporary files. This keeps +# schema, expression, Python, and shell validation while avoiding the deadlock +# tracked by rhysd/actionlint#712. + +require "json" +require "open3" +require "tempfile" +require "yaml" + +QUEUE_DIAGNOSTIC = + 'unexpected key "queue" for "concurrency" section\. expected one of "cancel-in-progress", "group"' +SHELLCHECK_EXCLUSIONS = "SC1091,SC2194,SC2050,SC2153,SC2154,SC2157,SC2043" + +class WorkflowLintError < StandardError; end + +def load_workflow(path) + document = YAML.safe_load( + File.read(path, encoding: "UTF-8"), + permitted_classes: [], + permitted_symbols: [], + aliases: true + ) + raise WorkflowLintError, "#{path}: workflow document must be a mapping" unless document.is_a?(Hash) + + document +rescue Psych::Exception, SystemCallError => error + raise WorkflowLintError, "#{path}: could not read workflow YAML: #{error.message}" +end + +def validate_concurrency_queue!(path, label, concurrency) + return unless concurrency.is_a?(Hash) && concurrency.key?("queue") + return if concurrency["queue"] == "max" + + raise WorkflowLintError, + "#{path}: #{label} concurrency queue must be exactly max, got #{concurrency['queue'].inspect}" +end + +def validate_queue_contract!(path, workflow) + validate_concurrency_queue!(path, "workflow", workflow["concurrency"]) + jobs = workflow["jobs"] + return unless jobs.is_a?(Hash) + + jobs.each do |job_name, job| + next unless job.is_a?(Hash) + + validate_concurrency_queue!(path, "job #{job_name}", job["concurrency"]) + end +end + +def windows_runner?(job) + Array(job["runs-on"]).any? do |label| + normalized = label.to_s.downcase + normalized == "windows" || normalized.start_with?("windows-") + end +end + +def effective_shell(workflow, job, step) + step["shell"] || + job.dig("defaults", "run", "shell") || + workflow.dig("defaults", "run", "shell") || + (windows_runner?(job) ? "pwsh" : "bash") +end + +def shellcheck_dialect(shell) + return shell if ["bash", "sh"].include?(shell) + return "bash" if shell.start_with?("bash ") + return "sh" if shell.start_with?("sh ") + + nil +end + +def sanitize_expressions(script) + sanitized = script.dup + offset = 0 + while (start_index = sanitized.index("${{", offset)) + end_index = sanitized.index("}}", start_index) + break unless end_index + + length = end_index + 2 - start_index + sanitized[start_index, length] = sanitized[start_index, length].gsub(/[^\r\n]/, "_") + offset = start_index + length + end + sanitized +end + +def shell_scripts(path, workflow) + jobs = workflow["jobs"] + return enum_for(__method__, path, workflow) unless block_given? + return unless jobs.is_a?(Hash) + + jobs.each do |job_name, job| + next unless job.is_a?(Hash) && job["steps"].is_a?(Array) + + job["steps"].each_with_index do |step, index| + next unless step.is_a?(Hash) && step["run"].is_a?(String) + + dialect = shellcheck_dialect(effective_shell(workflow, job, step).to_s) + next unless dialect + + step_name = step["name"].to_s.strip + step_name = (index + 1).to_s if step_name.empty? + yield path, job_name.to_s, step_name, dialect, step["run"] + end + end +end + +def run_actionlint(paths) + executable = ENV.fetch("ACTIONLINT", "actionlint") + arguments = [ + "-shellcheck=", + "-ignore", + QUEUE_DIAGNOSTIC, + *paths + ] + stdout, stderr, status = Open3.capture3(executable, *arguments) + return 0 if status.success? + + warn stdout unless stdout.empty? + warn stderr unless stderr.empty? + status.exitstatus || 2 +rescue SystemCallError => error + raise WorkflowLintError, "actionlint could not start: #{error.message}" +end + +def run_shellcheck(path, job_name, step_name, dialect, script) + setup = dialect == "bash" ? "set -eo pipefail" : "set -e" + source = "#{setup}\n#{sanitize_expressions(script)}\n" + executable = ENV.fetch("SHELLCHECK", "shellcheck") + stdout = stderr = nil + status = nil + + Tempfile.create(["actionlint-shellcheck-", ".#{dialect}"]) do |file| + file.chmod(0o600) + file.write(source) + file.flush + stdout, stderr, status = Open3.capture3( + executable, + "--norc", + "-f", + "json", + "-x", + "--shell", + dialect, + "-e", + SHELLCHECK_EXCLUSIONS, + file.path + ) + end + + unless [0, 1].include?(status.exitstatus) + detail = stderr.to_s.strip + detail = "exit #{status.exitstatus}" if detail.empty? + raise WorkflowLintError, "#{path}: ShellCheck failed for job=#{job_name} step=#{step_name}: #{detail}" + end + + findings = JSON.parse(stdout) + raise JSON::ParserError, "top-level result is not an array" unless findings.is_a?(Array) + + findings.each do |finding| + script_line = [finding.fetch("line").to_i - 1, 1].max + message = finding.fetch("message").to_s.delete_suffix(".") + warn( + "#{path}: shellcheck reported issue in job=#{job_name} step=#{step_name}: " \ + "SC#{finding.fetch('code')}:#{finding.fetch('level')}:#{script_line}:" \ + "#{finding.fetch('column')}: #{message}" + ) + end + findings.length +rescue JSON::ParserError, KeyError => error + raise WorkflowLintError, + "#{path}: invalid ShellCheck JSON for job=#{job_name} step=#{step_name}: #{error.message}" +rescue SystemCallError => error + raise WorkflowLintError, "ShellCheck could not start: #{error.message}" +end + +def lint(paths) + raise WorkflowLintError, "usage: lint_github_workflows.rb WORKFLOW..." if paths.empty? + + workflows = paths.to_h do |path| + workflow = load_workflow(path) + validate_queue_contract!(path, workflow) + [path, workflow] + end + actionlint_status = run_actionlint(paths) + return actionlint_status unless actionlint_status.zero? + + findings = workflows.sum do |path, workflow| + shell_scripts(path, workflow).sum do |script_path, job_name, step_name, dialect, script| + run_shellcheck(script_path, job_name, step_name, dialect, script) + end + end + findings.zero? ? 0 : 1 +rescue WorkflowLintError => error + warn "ERROR: #{error.message}" + 2 +end + +exit lint(ARGV) diff --git a/tests/test_exact_artifact_sbom_attestation_contract.py b/tests/test_exact_artifact_sbom_attestation_contract.py index 511f5cd24e..4b3574bbeb 100644 --- a/tests/test_exact_artifact_sbom_attestation_contract.py +++ b/tests/test_exact_artifact_sbom_attestation_contract.py @@ -247,6 +247,9 @@ def test_workflow_attests_each_exact_distribution_and_exports_offline_evidence() assert "offline-attestation-evidence/README.md" in signer assert "offline-attestation-evidence/SHA256SUMS" in signer assert "sha256sum" in signer + assert 'mapfile -t evidence_files < "$evidence_file_list"' in signer + assert 'LC_ALL=C sort > "$evidence_file_list"' in signer + assert "mapfile -t evidence_files < <(" not in signer def test_quality_workflow_pins_supported_runner_images() -> None: diff --git a/tests/test_lint_github_workflows.py b/tests/test_lint_github_workflows.py new file mode 100644 index 0000000000..b104d5cc5f --- /dev/null +++ b/tests/test_lint_github_workflows.py @@ -0,0 +1,307 @@ +"""Behavioral regressions for bounded actionlint and ShellCheck execution.""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +import shutil +import subprocess +import textwrap + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +LINTER = ROOT / "scripts" / "ci" / "lint_github_workflows.rb" +AUTOFIX_WORKFLOW = ROOT / ".github" / "workflows" / "pr-review-autofix.yml" + + +def _write_executable(path: Path, source: str) -> None: + """Write one executable test transport with deterministic behavior.""" + + path.write_text(textwrap.dedent(source).lstrip(), encoding="utf-8") + path.chmod(0o755) + + +def _tool_environment(tmp_path: Path) -> tuple[dict[str, str], Path]: + """Return isolated fake lint executables and their capture directory.""" + + binary_dir = tmp_path / "bin" + capture_dir = tmp_path / "captures" + binary_dir.mkdir() + capture_dir.mkdir() + _write_executable( + binary_dir / "actionlint", + """ + #!/usr/bin/env python3 + import json + import os + from pathlib import Path + import sys + + capture = Path(os.environ["LINT_CAPTURE_DIR"]) / "actionlint.json" + capture.write_text(json.dumps(sys.argv[1:]), encoding="utf-8") + print(os.environ.get("ACTIONLINT_OUTPUT", ""), end="") + raise SystemExit(int(os.environ.get("ACTIONLINT_STATUS", "0"))) + """, + ) + _write_executable( + binary_dir / "shellcheck", + """ + #!/usr/bin/env python3 + import json + import os + from pathlib import Path + import sys + + root = Path(os.environ["LINT_CAPTURE_DIR"]) + index = len(list(root.glob("shellcheck-*.json"))) + script = Path(sys.argv[-1]).read_text(encoding="utf-8") + (root / f"shellcheck-{index}.json").write_text( + json.dumps({"args": sys.argv[1:], "script": script}), + encoding="utf-8", + ) + if "FINDING_MARKER" in script: + print(json.dumps([{ + "line": 3, + "column": 7, + "level": "warning", + "code": 2086, + "message": "Double quote to prevent globbing.", + }])) + raise SystemExit(1) + if os.environ.get("SHELLCHECK_MALFORMED") == "1": + print("not-json") + raise SystemExit(0) + print("[]") + """, + ) + environment = { + **os.environ, + "PATH": f"{binary_dir}{os.pathsep}{os.environ['PATH']}", + "ACTIONLINT": str(binary_dir / "actionlint"), + "SHELLCHECK": str(binary_dir / "shellcheck"), + "LINT_CAPTURE_DIR": str(capture_dir), + } + return environment, capture_dir + + +def _run_linter(workflow: Path, environment: dict[str, str]) -> subprocess.CompletedProcess[str]: + """Run the real trusted linter against one controlled workflow.""" + + if shutil.which("ruby", path=environment["PATH"]) is None: + pytest.skip("Ruby is unavailable; the hosted quality job runs this runtime contract") + return subprocess.run( + ["ruby", str(LINTER), str(workflow)], + env=environment, + capture_output=True, + text=True, + check=False, + ) + + +def test_linter_uses_actionlint_schema_and_file_based_shellcheck(tmp_path: Path) -> None: + """Large Bash and explicit sh scripts use files while other shells stay excluded.""" + + environment, capture_dir = _tool_environment(tmp_path) + workflow = tmp_path / "large.yml" + large_body = "\n".join(" # bounded filler" for _ in range(4_000)) + workflow.write_text( + "\n".join( + ( + "name: large-shell-boundary", + "on: push", + "defaults:", + " run:", + " shell: bash", + "concurrency:", + " group: exact", + " queue: max", + "jobs:", + " linux:", + " runs-on: ubuntu-24.04", + " steps:", + " - name: Large Bash", + " run: |", + ' echo "${{ github.sha }}"', + large_body, + " - name: Explicit sh", + " shell: sh", + " run: echo ok", + " - name: Python", + " shell: python", + " run: print('ok')", + " windows:", + " runs-on: windows-2025", + " steps:", + " - shell: pwsh", + " run: Write-Host ok", + "", + ) + ), + encoding="utf-8", + ) + + result = _run_linter(workflow, environment) + + actionlint_args = json.loads( + (capture_dir / "actionlint.json").read_text(encoding="utf-8") + ) + shellcheck_records = [ + json.loads(path.read_text(encoding="utf-8")) + for path in sorted(capture_dir.glob("shellcheck-*.json")) + ] + assert result.returncode == 0, result.stderr + assert actionlint_args[0] == "-shellcheck=" + assert actionlint_args[-1] == str(workflow) + assert len(shellcheck_records) == 2 + assert shellcheck_records[0]["args"][:7] == [ + "--norc", + "-f", + "json", + "-x", + "--shell", + "bash", + "-e", + ] + assert shellcheck_records[0]["args"][-1] != "-" + assert shellcheck_records[0]["script"].startswith( + 'set -eo pipefail\necho "_________________"\n' + ) + assert len(shellcheck_records[0]["script"].encode()) > 65_536 + assert shellcheck_records[1]["args"][5] == "sh" + assert shellcheck_records[1]["script"] == "set -e\necho ok\n" + + +def test_linter_preserves_lines_inside_multiline_expressions(tmp_path: Path) -> None: + """Expression sanitizing keeps ShellCheck source and diagnostic lines aligned.""" + + environment, capture_dir = _tool_environment(tmp_path) + workflow = tmp_path / "multiline-expression.yml" + workflow.write_text( + """name: multiline-expression +on: push +jobs: + verify: + runs-on: ubuntu-24.04 + steps: + - run: | + echo "${{ + github.sha + }}" + echo after +""", + encoding="utf-8", + ) + + result = _run_linter(workflow, environment) + + record = json.loads( + (capture_dir / "shellcheck-0.json").read_text(encoding="utf-8") + ) + lines = record["script"].splitlines() + assert result.returncode == 0, result.stderr + assert lines[2].strip(" _") == "" + assert lines[3].endswith('"') + assert lines[4] == "echo after" + + +def test_write_capable_autofix_always_uses_the_trusted_linter() -> None: + """Changed workflows fail closed through the dispatch-pinned helper.""" + + workflow = AUTOFIX_WORKFLOW.read_text(encoding="utf-8") + invocation = ( + 'ruby "$GITHUB_WORKSPACE/trusted-autofix-source/scripts/ci/' + 'lint_github_workflows.rb"' + ) + + assert invocation in workflow + assert "command -v actionlint" not in workflow + + +def test_linter_reports_shellcheck_findings_with_workflow_context(tmp_path: Path) -> None: + """A delegated finding remains actionable without exposing a temporary filename.""" + + environment, _capture_dir = _tool_environment(tmp_path) + workflow = tmp_path / "finding.yml" + workflow.write_text( + """name: finding +on: push +jobs: + verify: + runs-on: ubuntu-24.04 + steps: + - name: Unsafe expansion + run: | + echo FINDING_MARKER +""", + encoding="utf-8", + ) + + result = _run_linter(workflow, environment) + + assert result.returncode == 1 + assert str(workflow) in result.stderr + assert "job=verify" in result.stderr + assert "step=Unsafe expansion" in result.stderr + assert "SC2086:warning:2:7" in result.stderr + assert "Double quote to prevent globbing" in result.stderr + + +def test_linter_rejects_unsupported_queue_before_actionlint(tmp_path: Path) -> None: + """The temporary actionlint exception cannot admit an invented queue value.""" + + environment, capture_dir = _tool_environment(tmp_path) + workflow = tmp_path / "bad-queue.yml" + workflow.write_text( + """name: bad-queue +on: push +concurrency: + group: exact + queue: newest +jobs: {} +""", + encoding="utf-8", + ) + + result = _run_linter(workflow, environment) + + assert result.returncode == 2 + assert "queue must be exactly max" in result.stderr + assert not (capture_dir / "actionlint.json").exists() + + +@pytest.mark.parametrize( + ("environment_update", "expected"), + ( + ({"ACTIONLINT_STATUS": "3", "ACTIONLINT_OUTPUT": "schema failure\n"}, "schema failure"), + ({"SHELLCHECK_MALFORMED": "1"}, "invalid ShellCheck JSON"), + ), +) +def test_linter_fails_closed_on_tool_failures( + tmp_path: Path, + environment_update: dict[str, str], + expected: str, +) -> None: + """Schema-process and result-integrity failures never become clean evidence.""" + + environment, _capture_dir = _tool_environment(tmp_path) + environment.update(environment_update) + workflow = tmp_path / "tool-failure.yml" + workflow.write_text( + """name: tool-failure +on: push +jobs: + verify: + runs-on: ubuntu-24.04 + steps: + - run: echo ok +""", + encoding="utf-8", + ) + + result = _run_linter(workflow, environment) + + assert result.returncode != 0 + assert expected in result.stderr diff --git a/tests/test_opencode_rust_coverage_toolchain_contract.py b/tests/test_opencode_rust_coverage_toolchain_contract.py index b1fd4a124e..ea80a2f68c 100644 --- a/tests/test_opencode_rust_coverage_toolchain_contract.py +++ b/tests/test_opencode_rust_coverage_toolchain_contract.py @@ -67,6 +67,19 @@ def test_trusted_coverage_image_provisions_verified_llvm_19_tools() -> None: assert 'RUN test -x "$LLVM_PROFDATA"\n' in dispatch +def test_software_vulkan_adapter_uses_stable_glob_order() -> None: + """Select the first matching lavapipe adapter in stable pathname order.""" + + dispatch = _dispatch_text() + adapter = dispatch.split("ensure_rust_gpu_adapter() {", 1)[1].split( + "\n }", 1 + )[0] + assert "for candidate in /usr/share/vulkan/icd.d/lvp_icd*.json; do" in adapter + assert 'if [ -f "$candidate" ]; then' in adapter + assert 'lvp_icd="$candidate"' in adapter + assert "-print -quit" not in adapter + + def test_isolated_runtime_receives_reviewed_llvm_constants() -> None: """Require exact LLVM 19 path constants at the Docker sandbox boundary.""" diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 00d91c1f5b..55a27a276f 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "3ba7c77db5dbe87b691b02c1da3d71178192f381" +REVIEW_DISPATCH_BLOB_SHA = "921c4a9b250912ec3f51516bbe7189ddd1034ed7" def _workflow_text(path: Path) -> str: From 53b53cfec2164eb653f31c446ebdb6875a8edee9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 20:03:43 +0900 Subject: [PATCH 08/14] fix(ci): preserve scheduler and Strix runtime evidence --- CHANGELOG.md | 10 ++++++- ...actionlint-modern-schema-and-shellcheck.md | 6 ++++ docs/doctoring/fork-head-review-dispatch.md | 6 ++++ .../strix-nvidia-nim-not-found-fallback.md | 19 ++++++++++-- scripts/ci/lint_github_workflows.rb | 6 ++-- scripts/ci/pr_review_merge_scheduler.py | 13 ++++----- scripts/ci/strix_quick_gate.sh | 4 +++ tests/test_agent_mention_sweep_regressions.py | 27 ++++++++++++++--- tests/test_lint_github_workflows.py | 13 +++++++-- tests/test_pr_review_merge_scheduler.py | 27 ++++++++++++++--- ...est_strix_nvidia_nim_not_found_fallback.py | 29 +++++++++++++++++-- 11 files changed, 131 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 196b713054..e28b993239 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,10 +65,18 @@ Semantic Versioning where the repository publishes a release. as its declared source, failing closed when it is missing or resolves to the workflow `github.token`; case-fold repository host comparisons so casing drift cannot select the wrong Actions credential or skip same-repository - stale-run cleanup. + stale-run cleanup, and render withheld-mutation guidance from the recorded + decision instead of re-reading mutable process credentials. - Publish only the sanitized cumulative Strix report tree, avoiding a later copy of relative scanner output that could reintroduce known internal warning text into uploaded security evidence. +- Translate the workflow-only `openai-direct/...` Strix fallback alias to + LiteLLM's documented `openai/...` provider prefix before invocation, so a + recoverable NVIDIA capacity failure can reach the configured cross-provider + fallback without being misclassified as a successful scan. +- Invoke actionlint and ShellCheck through fixed argv executable names supplied + by the pinned trusted `PATH`, removing unused environment-selected command + overrides while preserving the no-shell, file-backed lint boundary. - Retry configured Strix fallback models when the primary provider records a rate-limit or infrastructure failure only in its structured report log, and diff --git a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md index af5b255f18..4f44750cc1 100644 --- a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md +++ b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md @@ -31,6 +31,12 @@ regular temporary files. It parses ShellCheck JSON, restores the workflow job and step identity in every diagnostic, preserves findings as a failing status, and fails closed on malformed output or a missing executable. +The helper invokes the fixed `actionlint` and `shellcheck` executable names as +argv, never a repository- or environment-selected command and never a shell +string. The pinned hosted setup supplies those names through its trusted +`PATH`; behavioral tests use an isolated temporary `PATH` to prove the same +argv boundary without introducing a second executable-selection channel. + The autofix worker ignores only actionlint's exact released-schema diagnostic for the concurrency `queue` key. Before linting, it rejects every changed workflow whose `queue` value is not exactly `max`; therefore the compatibility diff --git a/docs/doctoring/fork-head-review-dispatch.md b/docs/doctoring/fork-head-review-dispatch.md index ab48713d0a..9839001308 100644 --- a/docs/doctoring/fork-head-review-dispatch.md +++ b/docs/doctoring/fork-head-review-dispatch.md @@ -121,6 +121,12 @@ same-repository stale-run cleanup. This is a zero-trust verification at the mutation boundary rather than trust in an upstream environment label (Rose et al., 2020). +The scheduler also records the credential refusal in each immutable decision +reason and renders later JSON and Actions guidance from that captured evidence. +It does not re-read mutable process credentials while serializing a decision, +so a surrounding test or caller cannot turn a valid wait into a summary-time +exception by changing the environment after inspection. + ## Draft merge defense in depth Exact-head Strix run `32573579932` reported `vuln-0001`, alleging that a draft diff --git a/docs/doctoring/strix-nvidia-nim-not-found-fallback.md b/docs/doctoring/strix-nvidia-nim-not-found-fallback.md index a088aa7ef8..b9a7459a59 100644 --- a/docs/doctoring/strix-nvidia-nim-not-found-fallback.md +++ b/docs/doctoring/strix-nvidia-nim-not-found-fallback.md @@ -5,8 +5,8 @@ Strix treats an authenticated NVIDIA NIM model-catalog `404 Not Found` as provider availability evidence, not as a target-application vulnerability. The gate does not retry the same unavailable model. It proceeds to a distinct -reviewed NVIDIA hosted model and only then to the existing GitHub Models -candidates. +reviewed NVIDIA hosted model and only then to the configured direct OpenAI +candidate. Public-repository scans now default to `nvidia/nemotron-3-super-120b-a12b`. The first fallback is @@ -30,6 +30,15 @@ combining with an unrelated application `404` to spoof infrastructure fallback. Provider-side failure also remains a fail-closed incomplete scan until a distinct fallback produces complete evidence. +[Required run 32632284647](https://github.com/ContextualWisdomLab/.github/actions/runs/32632284647) +showed why the model-name boundary must be explicit. After NVIDIA capacity +failures, the workflow's `openai-direct/gpt-5.6-luna` routing alias reached +LiteLLM unchanged, so LiteLLM rejected it because `openai-direct` is not a +provider prefix. The shared gate now translates that workflow-only alias to +LiteLLM's documented `openai/gpt-5.6-luna` form before Strix starts. This is a +transport-normalization repair, not a model-selection change or a provider +failure classified as a successful scan. + Exhausted provider infrastructure remains fail-closed even when the trusted gate has classified every observed threshold finding as outside the pull request's changed files. That classification scopes authoritative findings; it @@ -49,7 +58,8 @@ Regression evidence proves that: context is not recognized; 5. model-catalog 404s enter cross-model fallback but never same-model retry; 6. the primary and first fallback are current NVIDIA hosted models; -7. GitHub Models remain later cross-provider fallbacks; +7. the direct OpenAI workflow alias is translated to LiteLLM's `openai/` + provider prefix before invocation; 8. provider exhaustion remains non-passing after unchanged baseline findings; 9. changed, unmapped, and changed-manifest findings also block after provider exhaustion; and @@ -66,6 +76,9 @@ Strix severity, changed-file attribution, or independent approval requirements. ## References +BerriAI. (n.d.). *LiteLLM documentation*. Retrieved August 23, 2026, from +https://docs.litellm.ai/ + Fielding, R., Nottingham, M., & Reschke, J. (2022). *HTTP semantics* (RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 diff --git a/scripts/ci/lint_github_workflows.rb b/scripts/ci/lint_github_workflows.rb index 620c419980..3d82b178b2 100644 --- a/scripts/ci/lint_github_workflows.rb +++ b/scripts/ci/lint_github_workflows.rb @@ -109,14 +109,13 @@ def shell_scripts(path, workflow) end def run_actionlint(paths) - executable = ENV.fetch("ACTIONLINT", "actionlint") arguments = [ "-shellcheck=", "-ignore", QUEUE_DIAGNOSTIC, *paths ] - stdout, stderr, status = Open3.capture3(executable, *arguments) + stdout, stderr, status = Open3.capture3("actionlint", *arguments) return 0 if status.success? warn stdout unless stdout.empty? @@ -129,7 +128,6 @@ def run_actionlint(paths) def run_shellcheck(path, job_name, step_name, dialect, script) setup = dialect == "bash" ? "set -eo pipefail" : "set -e" source = "#{setup}\n#{sanitize_expressions(script)}\n" - executable = ENV.fetch("SHELLCHECK", "shellcheck") stdout = stderr = nil status = nil @@ -138,7 +136,7 @@ def run_shellcheck(path, job_name, step_name, dialect, script) file.write(source) file.flush stdout, stderr, status = Open3.capture3( - executable, + "shellcheck", "--norc", "-f", "json", diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 6914f83b10..d0fbc1168c 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -289,13 +289,10 @@ def require_workflow_starting_mutation_credential(action: str) -> None: raise RuntimeError(non_triggering_head_mutation_reason(action)) -def head_mutation_credential_guidance_text() -> tuple[str, str]: - """Return operator-facing summary and limit text for a withheld head mutation.""" - problem = head_mutation_credential_problem() - if problem is None: - raise RuntimeError("withheld-mutation messaging requires a non-triggering mutation credential") +def head_mutation_credential_guidance_text(withheld_reason: str) -> tuple[str, str]: + """Render operator guidance from the credential decision already recorded.""" return ( - f"The scheduler withheld a head mutation because {problem}.", + f"The scheduler withheld a head mutation. Recorded decision: {withheld_reason}", "Moving the head is unsafe until the scheduler can prove that the selected credential starts the required current-head workflow runs.", ) @@ -446,7 +443,7 @@ def decision_guidance(decision: Decision) -> dict[str, Any] | None: ], } if parse_non_triggering_head_mutation_reason(decision.reason): - summary, automation_limit = head_mutation_credential_guidance_text() + summary, automation_limit = head_mutation_credential_guidance_text(decision.reason) return { "type": "head_mutation_credential_upgrade", "token": mutation_token_label(), @@ -3121,7 +3118,7 @@ def head_mutation_credential_upgrade_summary(decisions: list[Decision]) -> list[ waits = [decision for decision in decisions if parse_non_triggering_head_mutation_reason(decision.reason)] if not waits: return [] - summary, automation_limit = head_mutation_credential_guidance_text() + summary, automation_limit = head_mutation_credential_guidance_text(waits[0].reason) lines = ["", "### Head mutation withheld", "", summary, automation_limit] lines.extend( [ diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 3373730017..4b10bc4c9a 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -2452,6 +2452,10 @@ child_model_for_api_base() { printf 'openai/%s\n' "${model#openai_direct/}" return 0 ;; + openai-direct/*) + printf 'openai/%s\n' "${model#openai-direct/}" + return 0 + ;; esac printf '%s\n' "$model" diff --git a/tests/test_agent_mention_sweep_regressions.py b/tests/test_agent_mention_sweep_regressions.py index 643f562cc0..a4c0ed21d9 100644 --- a/tests/test_agent_mention_sweep_regressions.py +++ b/tests/test_agent_mention_sweep_regressions.py @@ -4,6 +4,7 @@ import importlib import sys +import threading from datetime import datetime, timezone from pathlib import Path @@ -94,11 +95,29 @@ def test_pull_pagination_stops_at_cutoff_without_loading_later_pages() -> None: assert sweep.flatten_pages([{"number": 1}]) == [{"number": 1}] -def test_recent_pull_requests_use_bounded_parallel_repository_fetches(monkeypatch) -> None: - """Repository fetches are parallel but results remain repository ordered.""" +def test_recent_pull_requests_emit_bounded_parallel_fetches_as_they_finish( + monkeypatch, +) -> None: + """A slow repository cannot hide a completed sibling repository result.""" sweep = module() - client = PagingClient( + second_completed = threading.Event() + + class CompletionOrderClient(PagingClient): + """Hold the first repository until the second one has completed.""" + + def request(self, args, *, input_payload=None): + """Make repository completion order deterministic for the assertion.""" + + endpoint = args[0] + if endpoint == "repos/ContextualWisdomLab/first/pulls": + assert second_completed.wait(timeout=5) + response = super().request(args, input_payload=input_payload) + if endpoint == "repos/ContextualWisdomLab/second/pulls": + second_completed.set() + return response + + client = CompletionOrderClient( { ("orgs/ContextualWisdomLab/repos", 1): [[ repository("first"), @@ -129,8 +148,8 @@ def recording_executor(*, max_workers): ) ) assert [result["repository"] for result in results] == [ - "ContextualWisdomLab/first", "ContextualWisdomLab/second", + "ContextualWisdomLab/first", ] assert worker_limits == [2] diff --git a/tests/test_lint_github_workflows.py b/tests/test_lint_github_workflows.py index b104d5cc5f..93f90bc046 100644 --- a/tests/test_lint_github_workflows.py +++ b/tests/test_lint_github_workflows.py @@ -80,8 +80,6 @@ def _tool_environment(tmp_path: Path) -> tuple[dict[str, str], Path]: environment = { **os.environ, "PATH": f"{binary_dir}{os.pathsep}{os.environ['PATH']}", - "ACTIONLINT": str(binary_dir / "actionlint"), - "SHELLCHECK": str(binary_dir / "shellcheck"), "LINT_CAPTURE_DIR": str(capture_dir), } return environment, capture_dir @@ -220,6 +218,17 @@ def test_write_capable_autofix_always_uses_the_trusted_linter() -> None: assert "command -v actionlint" not in workflow +def test_linter_invokes_fixed_tool_names_without_dynamic_command_selection() -> None: + """Repository input cannot select the executable passed to Open3.""" + + source = LINTER.read_text(encoding="utf-8") + + assert 'ENV.fetch("ACTIONLINT"' not in source + assert 'ENV.fetch("SHELLCHECK"' not in source + assert 'Open3.capture3("actionlint", *arguments)' in source + assert 'Open3.capture3(\n "shellcheck",' in source + + def test_linter_reports_shellcheck_findings_with_workflow_context(tmp_path: Path) -> None: """A delegated finding remains actionable without exposing a temporary filename.""" diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index de8a998a16..d75175e419 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1847,14 +1847,33 @@ def test_workflow_starting_credentials_allow_head_mutations(monkeypatch): sched.require_workflow_starting_mutation_credential("update-branch") -def test_withheld_mutation_messages_reject_a_workflow_starting_credential(monkeypatch): - """Withheld-mutation helpers reject callers that have no credential problem.""" +def test_withheld_mutation_reason_rejects_a_workflow_starting_credential(monkeypatch): + """A safe credential cannot create a contradictory withheld-mutation reason.""" monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "PR_REVIEW_MERGE_TOKEN") with pytest.raises(RuntimeError, match="requires a non-triggering mutation credential"): sched.non_triggering_head_mutation_reason("update-branch") - with pytest.raises(RuntimeError, match="requires a non-triggering mutation credential"): - sched.head_mutation_credential_guidance_text() + + +def test_withheld_mutation_guidance_uses_recorded_reason_after_environment_changes( + monkeypatch, +): + """Render a captured wait decision without re-reading mutable token state.""" + monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "github-token") + reason = sched.non_triggering_head_mutation_reason("branch update") + + monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "PR_REVIEW_MERGE_TOKEN") + monkeypatch.setenv("GH_TOKEN", "selected-mutation-token") + monkeypatch.setenv("SCHEDULER_WORKFLOW_TOKEN", "workflow-runner-token") + assert sched.head_mutation_credential_starts_workflows() + + decision = sched.Decision(7, "wait", reason) + guidance = sched.decision_guidance(decision) + assert guidance is not None + assert "workflow GITHUB_TOKEN" in guidance["summary"] + assert "workflow GITHUB_TOKEN" in "\n".join( + sched.head_mutation_credential_upgrade_summary([decision]) + ) @pytest.mark.parametrize( diff --git a/tests/test_strix_nvidia_nim_not_found_fallback.py b/tests/test_strix_nvidia_nim_not_found_fallback.py index 9902697255..ceb87f1d60 100644 --- a/tests/test_strix_nvidia_nim_not_found_fallback.py +++ b/tests/test_strix_nvidia_nim_not_found_fallback.py @@ -2,8 +2,8 @@ The central Strix workflow must not turn a provider-side model-catalog 404 into a security finding or retry the same unavailable model. It must move to another -approved free NVIDIA NIM candidate before using the existing GitHub Models -fallbacks, while ordinary application 404 output remains non-retryable. +approved free NVIDIA NIM candidate before using the reviewed direct OpenAI +fallback, while ordinary application 404 output remains non-retryable. """ from __future__ import annotations @@ -187,7 +187,7 @@ def test_not_found_skips_same_model_and_enters_cross_model_fallback(self) -> Non self.assertNotIn("is_nvidia_nim_not_found_error", same_model_retry) def test_workflow_uses_available_free_first_nvidia_plan(self) -> None: - """Prefer a documented hosted NIM and another NIM before GitHub.""" + """Prefer a documented hosted NIM and another NIM before OpenAI.""" workflow = STRIX_WORKFLOW.read_text(encoding="utf-8") default_expression = ( @@ -213,6 +213,29 @@ def test_workflow_uses_available_free_first_nvidia_plan(self) -> None: )[0] self.assertNotIn(RETIRED_PRIMARY_MODEL, default_gate) + def test_direct_openai_fallback_alias_uses_litellm_provider_prefix(self) -> None: + """Translate the workflow alias before invoking LiteLLM through Strix.""" + + gate_source = STRIX_GATE.read_text(encoding="utf-8") + function_source = _function_block(gate_source, "child_model_for_api_base") + script = "\n".join( + ( + "set -euo pipefail", + "is_github_models_api_base() { return 1; }", + function_source, + 'child_model_for_api_base "$1" ""', + ) + ) + completed = subprocess.run( + ["bash", "-c", script, "strix-model", "openai-direct/gpt-5.6-luna"], + check=False, + capture_output=True, + text=True, + ) + + self.assertEqual(completed.returncode, 0, completed.stderr) + self.assertEqual(completed.stdout.strip(), "openai/gpt-5.6-luna") + def test_outer_workflow_requires_litellm_context_for_nvidia_404(self) -> None: """Reject provider-like target text in the outer neutralization gate.""" From a825ef211c87a325e8dc9c3e7d84d3a4b939d613 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 20:54:56 +0900 Subject: [PATCH 09/14] fix(ci): document fixed linter argv boundary --- CHANGELOG.md | 4 +++- .../doctoring/actionlint-modern-schema-and-shellcheck.md | 9 +++++++++ scripts/ci/lint_github_workflows.rb | 2 +- tests/test_lint_github_workflows.py | 3 +++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e28b993239..5d2fc22d65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,7 +76,9 @@ Semantic Versioning where the repository publishes a release. fallback without being misclassified as a successful scan. - Invoke actionlint and ShellCheck through fixed argv executable names supplied by the pinned trusted `PATH`, removing unused environment-selected command - overrides while preserving the no-shell, file-backed lint boundary. + overrides while preserving the no-shell, file-backed lint boundary; narrowly + suppress Semgrep's remaining false positive on the literal actionlint call, + whose dynamic workflow paths remain separate argv values. - Retry configured Strix fallback models when the primary provider records a rate-limit or infrastructure failure only in its structured report log, and diff --git a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md index 4f44750cc1..c6c5598d13 100644 --- a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md +++ b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md @@ -37,6 +37,15 @@ string. The pinned hosted setup supplies those names through its trusted `PATH`; behavioral tests use an isolated temporary `PATH` to prove the same argv boundary without introducing a second executable-selection channel. +[Required Semgrep run 32637664667](https://github.com/ContextualWisdomLab/.github/actions/runs/32637664667) +still classified the fixed `actionlint` invocation as dynamic because workflow +paths remain argv values. Ruby's `Open3.capture3` passes these separate +arguments directly to the literal executable and does not invoke a shell. The +single inline Semgrep suppression therefore applies only to that reviewed +false positive; the executable-name regression, isolated `PATH` execution, and +fail-closed actionlint status handling remain mandatory. The separate +ShellCheck invocation remains unsuppressed. + The autofix worker ignores only actionlint's exact released-schema diagnostic for the concurrency `queue` key. Before linting, it rejects every changed workflow whose `queue` value is not exactly `max`; therefore the compatibility diff --git a/scripts/ci/lint_github_workflows.rb b/scripts/ci/lint_github_workflows.rb index 3d82b178b2..506db67746 100644 --- a/scripts/ci/lint_github_workflows.rb +++ b/scripts/ci/lint_github_workflows.rb @@ -115,7 +115,7 @@ def run_actionlint(paths) QUEUE_DIAGNOSTIC, *paths ] - stdout, stderr, status = Open3.capture3("actionlint", *arguments) + stdout, stderr, status = Open3.capture3("actionlint", *arguments) # nosemgrep: ruby.lang.security.dangerous-exec.dangerous-exec return 0 if status.success? warn stdout unless stdout.empty? diff --git a/tests/test_lint_github_workflows.py b/tests/test_lint_github_workflows.py index 93f90bc046..271af235ab 100644 --- a/tests/test_lint_github_workflows.py +++ b/tests/test_lint_github_workflows.py @@ -227,6 +227,9 @@ def test_linter_invokes_fixed_tool_names_without_dynamic_command_selection() -> assert 'ENV.fetch("SHELLCHECK"' not in source assert 'Open3.capture3("actionlint", *arguments)' in source assert 'Open3.capture3(\n "shellcheck",' in source + assert source.count( + "# nosemgrep: ruby.lang.security.dangerous-exec.dangerous-exec" + ) == 1 def test_linter_reports_shellcheck_findings_with_workflow_context(tmp_path: Path) -> None: From 2046995aaec94bc258c0b897ace3bbd7c40a12b1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 21:15:12 +0900 Subject: [PATCH 10/14] fix(ci): use a permissible workflow shell parser --- .github/workflows/pr-review-autofix.yml | 11 +- CHANGELOG.md | 22 +-- ...actionlint-modern-schema-and-shellcheck.md | 57 +++++--- scripts/ci/lint_github_workflows.rb | 74 ++++------ tests/test_agent_mention_sweep_regressions.py | 24 ++-- tests/test_lint_github_workflows.py | 129 ++++++++++++------ 6 files changed, 177 insertions(+), 140 deletions(-) diff --git a/.github/workflows/pr-review-autofix.yml b/.github/workflows/pr-review-autofix.yml index 49f4b34aa4..e8e204f69a 100644 --- a/.github/workflows/pr-review-autofix.yml +++ b/.github/workflows/pr-review-autofix.yml @@ -510,7 +510,16 @@ jobs: python3 -m py_compile "${changed_python_files[@]}" fi if [ "${#changed_workflows[@]}" -gt 0 ]; then - ruby "$GITHUB_WORKSPACE/trusted-autofix-source/scripts/ci/lint_github_workflows.rb" \ + shfmt_path="${RUNNER_TEMP}/shfmt" + curl -fsSL \ + -o "$shfmt_path" \ + https://github.com/mvdan/sh/releases/download/v3.13.1/shfmt_v3.13.1_linux_amd64 + printf '%s %s\n' \ + 'fb096c5d1ac6beabbdbaa2874d025badb03ee07929f0c9ff67563ce8c75398b1' \ + "$shfmt_path" | sha256sum -c - + chmod 0755 "$shfmt_path" + PATH="${RUNNER_TEMP}:${PATH}" \ + ruby "$GITHUB_WORKSPACE/trusted-autofix-source/scripts/ci/lint_github_workflows.rb" \ "${changed_workflows[@]}" fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d2fc22d65..9fa29007c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,12 +55,12 @@ Semantic Versioning where the repository publishes a release. ### Fixed -- Separate actionlint schema/expression/Pyflakes validation from file-based - ShellCheck execution so workflow shell blocks larger than 64 KiB cannot - deadlock the write-capable autofix verifier, preserve actionlint's shell and - expression semantics, and narrowly accept GitHub's native - `concurrency.queue: max` while rejecting every other queue value until - upstream actionlint schema support is released. +- Separate actionlint schema/expression/Pyflakes validation from its deadlocking + ShellCheck transport, replace the newly added GPL dependency with the + checksum-pinned BSD-3-Clause shfmt 3.13.1 syntax parser, preserve effective + shell and expression semantics for workflow blocks larger than 64 KiB, and + accept GitHub's native `concurrency.queue: max` only when + `cancel-in-progress` is false or absent. - Bound head-mutation authorization to the actual selected `GH_TOKEN` as well as its declared source, failing closed when it is missing or resolves to the workflow `github.token`; case-fold repository host comparisons so casing @@ -74,11 +74,11 @@ Semantic Versioning where the repository publishes a release. LiteLLM's documented `openai/...` provider prefix before invocation, so a recoverable NVIDIA capacity failure can reach the configured cross-provider fallback without being misclassified as a successful scan. -- Invoke actionlint and ShellCheck through fixed argv executable names supplied - by the pinned trusted `PATH`, removing unused environment-selected command - overrides while preserving the no-shell, file-backed lint boundary; narrowly - suppress Semgrep's remaining false positive on the literal actionlint call, - whose dynamic workflow paths remain separate argv values. +- Invoke actionlint and shfmt through fixed executable names supplied by the + pinned trusted `PATH`, removing unused environment-selected command overrides + while preserving the no-shell lint boundary; narrowly suppress Semgrep's + remaining false positive on the literal actionlint call, whose dynamic + workflow paths remain separate argv values. - Retry configured Strix fallback models when the primary provider records a rate-limit or infrastructure failure only in its structured report log, and diff --git a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md index c6c5598d13..d019d9b1c2 100644 --- a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md +++ b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md @@ -1,4 +1,4 @@ -# Actionlint modern-schema and large-shell compatibility +# Actionlint modern-schema and permissive shell-parser compatibility Decision date: **2026-08-22** @@ -22,20 +22,21 @@ validation or shell analysis. ## Decision Keep actionlint as the schema, expression, and Pyflakes validator, but disable -only its ShellCheck subprocess integration with `-shellcheck=`. The trusted +its ShellCheck subprocess integration with `-shellcheck=`. The trusted `lint_github_workflows.rb` boundary uses Ruby's standard-library Psych parser to read the same YAML scalar values, reproduces actionlint 1.7.12's workflow/job/ -runner/step shell precedence, expression normalization, implicit shell setup, -and narrow rule exclusions, and invokes the installed ShellCheck against unique -regular temporary files. It parses ShellCheck JSON, restores the workflow job -and step identity in every diagnostic, preserves findings as a failing status, -and fails closed on malformed output or a missing executable. - -The helper invokes the fixed `actionlint` and `shellcheck` executable names as -argv, never a repository- or environment-selected command and never a shell -string. The pinned hosted setup supplies those names through its trusted -`PATH`; behavioral tests use an isolated temporary `PATH` to prove the same -argv boundary without introducing a second executable-selection channel. +runner/step shell precedence, expression normalization, and implicit shell +setup. It streams each Bash or POSIX sh program into shfmt 3.13.1's syntax-tree +JSON mode and fails closed on syntax errors, malformed JSON, or a missing +executable. + +shfmt is BSD-3-Clause, which satisfies the binding commercial/permissive +license policy; the newly introduced direct GPL-3.0-or-later ShellCheck +dependency has been removed. The write-capable worker downloads the official +Linux amd64 shfmt 3.13.1 release only when a workflow changed, verifies its +published SHA-256 digest, and exposes only that verified binary through the +step-local `PATH`. Behavioral tests use an isolated temporary `PATH` to prove +the same fixed executable and argv boundary. [Required Semgrep run 32637664667](https://github.com/ContextualWisdomLab/.github/actions/runs/32637664667) still classified the fixed `actionlint` invocation as dynamic because workflow @@ -43,27 +44,33 @@ paths remain argv values. Ruby's `Open3.capture3` passes these separate arguments directly to the literal executable and does not invoke a shell. The single inline Semgrep suppression therefore applies only to that reviewed false positive; the executable-name regression, isolated `PATH` execution, and -fail-closed actionlint status handling remain mandatory. The separate -ShellCheck invocation remains unsuppressed. +fail-closed actionlint status handling remain mandatory. shfmt uses only +literal command arguments and receives the governed shell source through +standard input, so it needs no scanner suppression. The autofix worker ignores only actionlint's exact released-schema diagnostic for the concurrency `queue` key. Before linting, it rejects every changed workflow whose `queue` value is not exactly `max`; therefore the compatibility -exception cannot admit an invented queue mode. +exception cannot admit an invented queue mode. GitHub permits `queue: max` only +when `cancel-in-progress` is false or absent, so a statically true cancellation +setting is also rejected at both workflow and job scope before actionlint runs. This is a temporary compatibility boundary. Remove the queue diagnostic exception after an actionlint release containing pull request 654 is pinned. -Remove the stdin spool only after issue 712 is fixed and a greater-than-64-KiB -regression passes directly through the pinned actionlint/ShellCheck pair. +Remove the shfmt parser boundary only after issue 712 is fixed, actionlint ships +the corrected transport, its effective shell dependency satisfies the binding +license policy, and a greater-than-64-KiB regression passes through that +replacement. ## Verification -- A greater-than-64-KiB synthetic shell program reaches the delegated - ShellCheck executable through a regular file, without content loss. +- A greater-than-64-KiB synthetic shell program reaches the delegated shfmt + parser through the bounded Ruby subprocess transport without content loss. - Bash, sh, Windows/PowerShell, Python, workflow defaults, and GitHub expression normalization retain actionlint's effective-shell behavior. -- ShellCheck findings, malformed result JSON, actionlint failures, and invalid - concurrency queue values all fail closed with actionable workflow context. +- shfmt syntax failures, malformed result JSON, actionlint failures, invalid + concurrency queue values, and `queue: max` plus static cancellation all fail + closed with actionable workflow context. - The offline Python-only coverage sandbox records the Ruby subprocess contracts as unavailable instead of failing with `FileNotFoundError`; the hosted quality job, whose runner includes Ruby, executes those contracts and @@ -74,6 +81,12 @@ regression passes directly through the pinned actionlint/ShellCheck pair. GitHub. (2026, May 7). *GitHub Actions concurrency groups now allow larger queues*. https://github.blog/changelog/2026-05-07-github-actions-concurrency-groups-now-allow-larger-queues/ +Martí, D. (2026, April 6). *shfmt v3.13.1* [Computer software]. GitHub. +https://github.com/mvdan/sh/releases/tag/v3.13.1 + +Martí, D. (n.d.). *mvdan/sh license* [BSD 3-Clause license]. GitHub. Retrieved +August 23, 2026, from https://github.com/mvdan/sh/blob/master/LICENSE + Murai, R. (2025). *Support queue: max in concurrency* [Pull request #654]. GitHub. https://github.com/rhysd/actionlint/pull/654 diff --git a/scripts/ci/lint_github_workflows.rb b/scripts/ci/lint_github_workflows.rb index 506db67746..551e0cd1c1 100644 --- a/scripts/ci/lint_github_workflows.rb +++ b/scripts/ci/lint_github_workflows.rb @@ -1,20 +1,17 @@ #!/usr/bin/env ruby # frozen_string_literal: true -# Run actionlint without its oversized-stdin ShellCheck transport, then apply -# the same ShellCheck policy directly to regular temporary files. This keeps -# schema, expression, Python, and shell validation while avoiding the deadlock -# tracked by rhysd/actionlint#712. +# Run actionlint without its oversized-stdin ShellCheck transport, then parse +# shell steps with the permissively licensed shfmt parser. This keeps schema, +# expression, Python, and shell-syntax validation without the transport +# deadlock tracked by rhysd/actionlint#712 or a GPL tool dependency. require "json" require "open3" -require "tempfile" require "yaml" QUEUE_DIAGNOSTIC = 'unexpected key "queue" for "concurrency" section\. expected one of "cancel-in-progress", "group"' -SHELLCHECK_EXCLUSIONS = "SC1091,SC2194,SC2050,SC2153,SC2154,SC2157,SC2043" - class WorkflowLintError < StandardError; end def load_workflow(path) @@ -33,10 +30,14 @@ def load_workflow(path) def validate_concurrency_queue!(path, label, concurrency) return unless concurrency.is_a?(Hash) && concurrency.key?("queue") - return if concurrency["queue"] == "max" + unless concurrency["queue"] == "max" + raise WorkflowLintError, + "#{path}: #{label} concurrency queue must be exactly max, got #{concurrency['queue'].inspect}" + end + return unless concurrency["cancel-in-progress"] == true raise WorkflowLintError, - "#{path}: #{label} concurrency queue must be exactly max, got #{concurrency['queue'].inspect}" + "#{path}: #{label} concurrency queue max requires cancel-in-progress to be false or absent" end def validate_queue_contract!(path, workflow) @@ -125,54 +126,31 @@ def run_actionlint(paths) raise WorkflowLintError, "actionlint could not start: #{error.message}" end -def run_shellcheck(path, job_name, step_name, dialect, script) +def run_shfmt(path, job_name, step_name, dialect, script) setup = dialect == "bash" ? "set -eo pipefail" : "set -e" source = "#{setup}\n#{sanitize_expressions(script)}\n" - stdout = stderr = nil - status = nil - - Tempfile.create(["actionlint-shellcheck-", ".#{dialect}"]) do |file| - file.chmod(0o600) - file.write(source) - file.flush - stdout, stderr, status = Open3.capture3( - "shellcheck", - "--norc", - "-f", - "json", - "-x", - "--shell", - dialect, - "-e", - SHELLCHECK_EXCLUSIONS, - file.path - ) + stdout, stderr, status = if dialect == "bash" + Open3.capture3("shfmt", "-ln", "bash", "-tojson", stdin_data: source) + else + Open3.capture3("shfmt", "-ln", "posix", "-tojson", stdin_data: source) end - unless [0, 1].include?(status.exitstatus) + unless status.success? detail = stderr.to_s.strip detail = "exit #{status.exitstatus}" if detail.empty? - raise WorkflowLintError, "#{path}: ShellCheck failed for job=#{job_name} step=#{step_name}: #{detail}" + raise WorkflowLintError, + "#{path}: shfmt could not parse job=#{job_name} step=#{step_name}: #{detail}" end - findings = JSON.parse(stdout) - raise JSON::ParserError, "top-level result is not an array" unless findings.is_a?(Array) - - findings.each do |finding| - script_line = [finding.fetch("line").to_i - 1, 1].max - message = finding.fetch("message").to_s.delete_suffix(".") - warn( - "#{path}: shellcheck reported issue in job=#{job_name} step=#{step_name}: " \ - "SC#{finding.fetch('code')}:#{finding.fetch('level')}:#{script_line}:" \ - "#{finding.fetch('column')}: #{message}" - ) - end - findings.length -rescue JSON::ParserError, KeyError => error + syntax_tree = JSON.parse(stdout) + raise JSON::ParserError, "top-level result is not an object" unless syntax_tree.is_a?(Hash) + + 0 +rescue JSON::ParserError => error raise WorkflowLintError, - "#{path}: invalid ShellCheck JSON for job=#{job_name} step=#{step_name}: #{error.message}" + "#{path}: invalid shfmt JSON for job=#{job_name} step=#{step_name}: #{error.message}" rescue SystemCallError => error - raise WorkflowLintError, "ShellCheck could not start: #{error.message}" + raise WorkflowLintError, "shfmt could not start: #{error.message}" end def lint(paths) @@ -188,7 +166,7 @@ def lint(paths) findings = workflows.sum do |path, workflow| shell_scripts(path, workflow).sum do |script_path, job_name, step_name, dialect, script| - run_shellcheck(script_path, job_name, step_name, dialect, script) + run_shfmt(script_path, job_name, step_name, dialect, script) end end findings.zero? ? 0 : 1 diff --git a/tests/test_agent_mention_sweep_regressions.py b/tests/test_agent_mention_sweep_regressions.py index a4c0ed21d9..4f5c171896 100644 --- a/tests/test_agent_mention_sweep_regressions.py +++ b/tests/test_agent_mention_sweep_regressions.py @@ -101,7 +101,7 @@ def test_recent_pull_requests_emit_bounded_parallel_fetches_as_they_finish( """A slow repository cannot hide a completed sibling repository result.""" sweep = module() - second_completed = threading.Event() + second_observed = threading.Event() class CompletionOrderClient(PagingClient): """Hold the first repository until the second one has completed.""" @@ -111,11 +111,8 @@ def request(self, args, *, input_payload=None): endpoint = args[0] if endpoint == "repos/ContextualWisdomLab/first/pulls": - assert second_completed.wait(timeout=5) - response = super().request(args, input_payload=input_payload) - if endpoint == "repos/ContextualWisdomLab/second/pulls": - second_completed.set() - return response + assert second_observed.wait(timeout=30) + return super().request(args, input_payload=input_payload) client = CompletionOrderClient( { @@ -139,14 +136,15 @@ def recording_executor(*, max_workers): "ThreadPoolExecutor", recording_executor, ) - results = list( - sweep.list_recent_pull_requests( - client, - organization="ContextualWisdomLab", - repository_source="organization", - since="2026-08-05T00:00:00Z", - ) + issues = sweep.list_recent_pull_requests( + client, + organization="ContextualWisdomLab", + repository_source="organization", + since="2026-08-05T00:00:00Z", ) + first_result = next(issues) + second_observed.set() + results = [first_result, *issues] assert [result["repository"] for result in results] == [ "ContextualWisdomLab/second", "ContextualWisdomLab/first", diff --git a/tests/test_lint_github_workflows.py b/tests/test_lint_github_workflows.py index 271af235ab..0d76f24e24 100644 --- a/tests/test_lint_github_workflows.py +++ b/tests/test_lint_github_workflows.py @@ -1,4 +1,4 @@ -"""Behavioral regressions for bounded actionlint and ShellCheck execution.""" +"""Behavioral regressions for bounded actionlint and shfmt execution.""" from __future__ import annotations @@ -47,7 +47,7 @@ def _tool_environment(tmp_path: Path) -> tuple[dict[str, str], Path]: """, ) _write_executable( - binary_dir / "shellcheck", + binary_dir / "shfmt", """ #!/usr/bin/env python3 import json @@ -56,25 +56,19 @@ def _tool_environment(tmp_path: Path) -> tuple[dict[str, str], Path]: import sys root = Path(os.environ["LINT_CAPTURE_DIR"]) - index = len(list(root.glob("shellcheck-*.json"))) - script = Path(sys.argv[-1]).read_text(encoding="utf-8") - (root / f"shellcheck-{index}.json").write_text( + index = len(list(root.glob("shfmt-*.json"))) + script = sys.stdin.read() + (root / f"shfmt-{index}.json").write_text( json.dumps({"args": sys.argv[1:], "script": script}), encoding="utf-8", ) - if "FINDING_MARKER" in script: - print(json.dumps([{ - "line": 3, - "column": 7, - "level": "warning", - "code": 2086, - "message": "Double quote to prevent globbing.", - }])) - raise SystemExit(1) - if os.environ.get("SHELLCHECK_MALFORMED") == "1": + if "SYNTAX_ERROR_MARKER" in script: + print("standard input:2:7: expected command", file=sys.stderr) + raise SystemExit(3) + if os.environ.get("SHFMT_MALFORMED") == "1": print("not-json") raise SystemExit(0) - print("[]") + print("{}") """, ) environment = { @@ -99,8 +93,8 @@ def _run_linter(workflow: Path, environment: dict[str, str]) -> subprocess.Compl ) -def test_linter_uses_actionlint_schema_and_file_based_shellcheck(tmp_path: Path) -> None: - """Large Bash and explicit sh scripts use files while other shells stay excluded.""" +def test_linter_uses_actionlint_schema_and_bounded_shfmt_parser(tmp_path: Path) -> None: + """Large Bash and explicit sh scripts reach shfmt without content loss.""" environment, capture_dir = _tool_environment(tmp_path) workflow = tmp_path / "large.yml" @@ -116,6 +110,7 @@ def test_linter_uses_actionlint_schema_and_file_based_shellcheck(tmp_path: Path) "concurrency:", " group: exact", " queue: max", + " cancel-in-progress: false", "jobs:", " linux:", " runs-on: ubuntu-24.04", @@ -146,30 +141,21 @@ def test_linter_uses_actionlint_schema_and_file_based_shellcheck(tmp_path: Path) actionlint_args = json.loads( (capture_dir / "actionlint.json").read_text(encoding="utf-8") ) - shellcheck_records = [ + shfmt_records = [ json.loads(path.read_text(encoding="utf-8")) - for path in sorted(capture_dir.glob("shellcheck-*.json")) + for path in sorted(capture_dir.glob("shfmt-*.json")) ] assert result.returncode == 0, result.stderr assert actionlint_args[0] == "-shellcheck=" assert actionlint_args[-1] == str(workflow) - assert len(shellcheck_records) == 2 - assert shellcheck_records[0]["args"][:7] == [ - "--norc", - "-f", - "json", - "-x", - "--shell", - "bash", - "-e", - ] - assert shellcheck_records[0]["args"][-1] != "-" - assert shellcheck_records[0]["script"].startswith( + assert len(shfmt_records) == 2 + assert shfmt_records[0]["args"] == ["-ln", "bash", "-tojson"] + assert shfmt_records[0]["script"].startswith( 'set -eo pipefail\necho "_________________"\n' ) - assert len(shellcheck_records[0]["script"].encode()) > 65_536 - assert shellcheck_records[1]["args"][5] == "sh" - assert shellcheck_records[1]["script"] == "set -e\necho ok\n" + assert len(shfmt_records[0]["script"].encode()) > 65_536 + assert shfmt_records[1]["args"] == ["-ln", "posix", "-tojson"] + assert shfmt_records[1]["script"] == "set -e\necho ok\n" def test_linter_preserves_lines_inside_multiline_expressions(tmp_path: Path) -> None: @@ -196,7 +182,7 @@ def test_linter_preserves_lines_inside_multiline_expressions(tmp_path: Path) -> result = _run_linter(workflow, environment) record = json.loads( - (capture_dir / "shellcheck-0.json").read_text(encoding="utf-8") + (capture_dir / "shfmt-0.json").read_text(encoding="utf-8") ) lines = record["script"].splitlines() assert result.returncode == 0, result.stderr @@ -224,16 +210,29 @@ def test_linter_invokes_fixed_tool_names_without_dynamic_command_selection() -> source = LINTER.read_text(encoding="utf-8") assert 'ENV.fetch("ACTIONLINT"' not in source - assert 'ENV.fetch("SHELLCHECK"' not in source + assert 'ENV.fetch("SHFMT"' not in source assert 'Open3.capture3("actionlint", *arguments)' in source - assert 'Open3.capture3(\n "shellcheck",' in source + assert 'Open3.capture3("shfmt", "-ln", "bash", "-tojson"' in source + assert 'Open3.capture3("shfmt", "-ln", "posix", "-tojson"' in source assert source.count( "# nosemgrep: ruby.lang.security.dangerous-exec.dangerous-exec" ) == 1 -def test_linter_reports_shellcheck_findings_with_workflow_context(tmp_path: Path) -> None: - """A delegated finding remains actionable without exposing a temporary filename.""" +def test_linter_uses_permissive_pinned_shfmt_instead_of_shellcheck() -> None: + """The write-capable linter must not add a GPL tool dependency.""" + + source = LINTER.read_text(encoding="utf-8") + workflow = AUTOFIX_WORKFLOW.read_text(encoding="utf-8") + + assert 'Open3.capture3("shfmt",' in source + assert 'Open3.capture3("shellcheck",' not in source + assert "shfmt_v3.13.1_linux_amd64" in workflow + assert "fb096c5d1ac6beabbdbaa2874d025badb03ee07929f0c9ff67563ce8c75398b1" in workflow + + +def test_linter_reports_shfmt_syntax_failures_with_workflow_context(tmp_path: Path) -> None: + """A parser failure identifies the governed workflow job and step.""" environment, _capture_dir = _tool_environment(tmp_path) workflow = tmp_path / "finding.yml" @@ -246,19 +245,18 @@ def test_linter_reports_shellcheck_findings_with_workflow_context(tmp_path: Path steps: - name: Unsafe expansion run: | - echo FINDING_MARKER + echo SYNTAX_ERROR_MARKER """, encoding="utf-8", ) result = _run_linter(workflow, environment) - assert result.returncode == 1 + assert result.returncode == 2 assert str(workflow) in result.stderr assert "job=verify" in result.stderr assert "step=Unsafe expansion" in result.stderr - assert "SC2086:warning:2:7" in result.stderr - assert "Double quote to prevent globbing" in result.stderr + assert "standard input:2:7: expected command" in result.stderr def test_linter_rejects_unsupported_queue_before_actionlint(tmp_path: Path) -> None: @@ -284,11 +282,52 @@ def test_linter_rejects_unsupported_queue_before_actionlint(tmp_path: Path) -> N assert not (capture_dir / "actionlint.json").exists() +@pytest.mark.parametrize( + "workflow_source", + ( + """name: cancelled-workflow-queue +on: push +concurrency: + group: exact + queue: max + cancel-in-progress: true +jobs: {} +""", + """name: cancelled-job-queue +on: push +jobs: + verify: + runs-on: ubuntu-24.04 + concurrency: + group: exact + queue: max + cancel-in-progress: true + steps: [] +""", + ), +) +def test_linter_rejects_queue_max_with_static_cancellation( + tmp_path: Path, + workflow_source: str, +) -> None: + """GitHub permits an expanded queue only when cancellation is disabled.""" + + environment, capture_dir = _tool_environment(tmp_path) + workflow = tmp_path / "cancelled-queue.yml" + workflow.write_text(workflow_source, encoding="utf-8") + + result = _run_linter(workflow, environment) + + assert result.returncode == 2 + assert "queue max requires cancel-in-progress to be false or absent" in result.stderr + assert not (capture_dir / "actionlint.json").exists() + + @pytest.mark.parametrize( ("environment_update", "expected"), ( ({"ACTIONLINT_STATUS": "3", "ACTIONLINT_OUTPUT": "schema failure\n"}, "schema failure"), - ({"SHELLCHECK_MALFORMED": "1"}, "invalid ShellCheck JSON"), + ({"SHFMT_MALFORMED": "1"}, "invalid shfmt JSON"), ), ) def test_linter_fails_closed_on_tool_failures( From e9001ce4c05467096728d70fcb89de70060d3395 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 21:45:35 +0900 Subject: [PATCH 11/14] fix(ci): provision trusted actionlint runtime --- .github/workflows/pr-review-autofix.yml | 11 ++++++++++- CHANGELOG.md | 12 +++++++----- .../actionlint-modern-schema-and-shellcheck.md | 18 +++++++++++++----- tests/test_lint_github_workflows.py | 14 ++++++++++++-- 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/.github/workflows/pr-review-autofix.yml b/.github/workflows/pr-review-autofix.yml index e8e204f69a..169790d567 100644 --- a/.github/workflows/pr-review-autofix.yml +++ b/.github/workflows/pr-review-autofix.yml @@ -510,14 +510,23 @@ jobs: python3 -m py_compile "${changed_python_files[@]}" fi if [ "${#changed_workflows[@]}" -gt 0 ]; then + actionlint_archive="${RUNNER_TEMP}/actionlint.tar.gz" + actionlint_path="${RUNNER_TEMP}/actionlint" shfmt_path="${RUNNER_TEMP}/shfmt" + curl -fsSL \ + -o "$actionlint_archive" \ + https://github.com/rhysd/actionlint/releases/download/v1.7.12/actionlint_1.7.12_linux_amd64.tar.gz + printf '%s %s\n' \ + '8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8' \ + "$actionlint_archive" | sha256sum -c - + tar -xzf "$actionlint_archive" -C "$RUNNER_TEMP" actionlint curl -fsSL \ -o "$shfmt_path" \ https://github.com/mvdan/sh/releases/download/v3.13.1/shfmt_v3.13.1_linux_amd64 printf '%s %s\n' \ 'fb096c5d1ac6beabbdbaa2874d025badb03ee07929f0c9ff67563ce8c75398b1' \ "$shfmt_path" | sha256sum -c - - chmod 0755 "$shfmt_path" + chmod 0755 "$actionlint_path" "$shfmt_path" PATH="${RUNNER_TEMP}:${PATH}" \ ruby "$GITHUB_WORKSPACE/trusted-autofix-source/scripts/ci/lint_github_workflows.rb" \ "${changed_workflows[@]}" diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa29007c8..79d4625dc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,11 +74,13 @@ Semantic Versioning where the repository publishes a release. LiteLLM's documented `openai/...` provider prefix before invocation, so a recoverable NVIDIA capacity failure can reach the configured cross-provider fallback without being misclassified as a successful scan. -- Invoke actionlint and shfmt through fixed executable names supplied by the - pinned trusted `PATH`, removing unused environment-selected command overrides - while preserving the no-shell lint boundary; narrowly suppress Semgrep's - remaining false positive on the literal actionlint call, whose dynamic - workflow paths remain separate argv values. +- Install actionlint 1.7.12 and shfmt 3.13.1 from their official release + artifacts with exact SHA-256 verification, then invoke both through fixed + executable names supplied by the trusted step-local `PATH`; this removes the + undocumented runner-image assumption and unused environment-selected command + overrides while preserving fail-closed, no-shell linting. Narrowly suppress + Semgrep's remaining false positive on the literal actionlint call, whose + dynamic workflow paths remain separate argv values. - Retry configured Strix fallback models when the primary provider records a rate-limit or infrastructure failure only in its structured report log, and diff --git a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md index d019d9b1c2..63a8c89e0f 100644 --- a/docs/doctoring/actionlint-modern-schema-and-shellcheck.md +++ b/docs/doctoring/actionlint-modern-schema-and-shellcheck.md @@ -33,10 +33,13 @@ executable. shfmt is BSD-3-Clause, which satisfies the binding commercial/permissive license policy; the newly introduced direct GPL-3.0-or-later ShellCheck dependency has been removed. The write-capable worker downloads the official -Linux amd64 shfmt 3.13.1 release only when a workflow changed, verifies its -published SHA-256 digest, and exposes only that verified binary through the -step-local `PATH`. Behavioral tests use an isolated temporary `PATH` to prove -the same fixed executable and argv boundary. +Linux amd64 actionlint 1.7.12 archive and shfmt 3.13.1 binary only when a +workflow changed, verifies both published SHA-256 digests, extracts only the +actionlint executable, and exposes only those verified executables through the +step-local `PATH`. This avoids relying on an undocumented runner-image tool +inventory while preserving fail-closed schema validation. Behavioral tests use +an isolated temporary `PATH` to prove the same fixed executable and argv +boundary. [Required Semgrep run 32637664667](https://github.com/ContextualWisdomLab/.github/actions/runs/32637664667) still classified the fixed `actionlint` invocation as dynamic because workflow @@ -74,7 +77,9 @@ replacement. - The offline Python-only coverage sandbox records the Ruby subprocess contracts as unavailable instead of failing with `FileNotFoundError`; the hosted quality job, whose runner includes Ruby, executes those contracts and - the real all-workflow lint command. + the real all-workflow lint command. The write-capable runtime does not assume + that actionlint is preinstalled: its exact release archive is checksum-pinned + beside shfmt before the linter starts. ## References @@ -90,5 +95,8 @@ August 23, 2026, from https://github.com/mvdan/sh/blob/master/LICENSE Murai, R. (2025). *Support queue: max in concurrency* [Pull request #654]. GitHub. https://github.com/rhysd/actionlint/pull/654 +Murai, R. (2026, March 30). *actionlint v1.7.12* [Computer software]. GitHub. +https://github.com/rhysd/actionlint/releases/tag/v1.7.12 + Murai, R. (2026). *Shellcheck integration deadlocks for run blocks greater than 64 KiB* [Issue #712]. GitHub. https://github.com/rhysd/actionlint/issues/712 diff --git a/tests/test_lint_github_workflows.py b/tests/test_lint_github_workflows.py index 0d76f24e24..2b4c800f2a 100644 --- a/tests/test_lint_github_workflows.py +++ b/tests/test_lint_github_workflows.py @@ -159,7 +159,7 @@ def test_linter_uses_actionlint_schema_and_bounded_shfmt_parser(tmp_path: Path) def test_linter_preserves_lines_inside_multiline_expressions(tmp_path: Path) -> None: - """Expression sanitizing keeps ShellCheck source and diagnostic lines aligned.""" + """Expression sanitizing keeps shfmt source and diagnostic lines aligned.""" environment, capture_dir = _tool_environment(tmp_path) workflow = tmp_path / "multiline-expression.yml" @@ -192,7 +192,7 @@ def test_linter_preserves_lines_inside_multiline_expressions(tmp_path: Path) -> def test_write_capable_autofix_always_uses_the_trusted_linter() -> None: - """Changed workflows fail closed through the dispatch-pinned helper.""" + """Changed workflows use checksum-pinned tools and the trusted helper.""" workflow = AUTOFIX_WORKFLOW.read_text(encoding="utf-8") invocation = ( @@ -202,6 +202,16 @@ def test_write_capable_autofix_always_uses_the_trusted_linter() -> None: assert invocation in workflow assert "command -v actionlint" not in workflow + assert "actionlint_1.7.12_linux_amd64.tar.gz" in workflow + assert ( + "8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8" + in workflow + ) + assert ( + 'tar -xzf "$actionlint_archive" -C "$RUNNER_TEMP" actionlint' + in workflow + ) + assert 'PATH="${RUNNER_TEMP}:${PATH}"' in workflow def test_linter_invokes_fixed_tool_names_without_dynamic_command_selection() -> None: From 6f19a6527649885d1df1615a3c01b639437a2081 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 21:58:28 +0900 Subject: [PATCH 12/14] refactor(ci): clarify shell parser contract --- CHANGELOG.md | 7 ++++--- scripts/ci/lint_github_workflows.rb | 6 +++--- tests/test_lint_github_workflows.py | 1 + 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79d4625dc1..77f8f039ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,9 +78,10 @@ Semantic Versioning where the repository publishes a release. artifacts with exact SHA-256 verification, then invoke both through fixed executable names supplied by the trusted step-local `PATH`; this removes the undocumented runner-image assumption and unused environment-selected command - overrides while preserving fail-closed, no-shell linting. Narrowly suppress - Semgrep's remaining false positive on the literal actionlint call, whose - dynamic workflow paths remain separate argv values. + overrides while preserving fail-closed, no-shell linting. Treat shfmt as the + parser contract it is instead of accumulating an unreachable findings count. + Narrowly suppress Semgrep's remaining false positive on the literal + actionlint call, whose dynamic workflow paths remain separate argv values. - Retry configured Strix fallback models when the primary provider records a rate-limit or infrastructure failure only in its structured report log, and diff --git a/scripts/ci/lint_github_workflows.rb b/scripts/ci/lint_github_workflows.rb index 551e0cd1c1..a16b12fc10 100644 --- a/scripts/ci/lint_github_workflows.rb +++ b/scripts/ci/lint_github_workflows.rb @@ -164,12 +164,12 @@ def lint(paths) actionlint_status = run_actionlint(paths) return actionlint_status unless actionlint_status.zero? - findings = workflows.sum do |path, workflow| - shell_scripts(path, workflow).sum do |script_path, job_name, step_name, dialect, script| + workflows.each do |path, workflow| + shell_scripts(path, workflow).each do |script_path, job_name, step_name, dialect, script| run_shfmt(script_path, job_name, step_name, dialect, script) end end - findings.zero? ? 0 : 1 + 0 rescue WorkflowLintError => error warn "ERROR: #{error.message}" 2 diff --git a/tests/test_lint_github_workflows.py b/tests/test_lint_github_workflows.py index 2b4c800f2a..e1deb375e7 100644 --- a/tests/test_lint_github_workflows.py +++ b/tests/test_lint_github_workflows.py @@ -224,6 +224,7 @@ def test_linter_invokes_fixed_tool_names_without_dynamic_command_selection() -> assert 'Open3.capture3("actionlint", *arguments)' in source assert 'Open3.capture3("shfmt", "-ln", "bash", "-tojson"' in source assert 'Open3.capture3("shfmt", "-ln", "posix", "-tojson"' in source + assert "findings = workflows.sum" not in source assert source.count( "# nosemgrep: ruby.lang.security.dangerous-exec.dangerous-exec" ) == 1 From 7b16617af04431a43f8f7528b8ac7db345e404a7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 22:35:28 +0900 Subject: [PATCH 13/14] fix(ci): preserve Strix repair ownership --- CHANGELOG.md | 4 ---- .../strix-nvidia-nim-not-found-fallback.md | 23 ++++++++----------- scripts/ci/strix_quick_gate.sh | 4 ---- ...est_strix_nvidia_nim_not_found_fallback.py | 23 ------------------- 4 files changed, 10 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77f8f039ce..c4f59620e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,10 +70,6 @@ Semantic Versioning where the repository publishes a release. - Publish only the sanitized cumulative Strix report tree, avoiding a later copy of relative scanner output that could reintroduce known internal warning text into uploaded security evidence. -- Translate the workflow-only `openai-direct/...` Strix fallback alias to - LiteLLM's documented `openai/...` provider prefix before invocation, so a - recoverable NVIDIA capacity failure can reach the configured cross-provider - fallback without being misclassified as a successful scan. - Install actionlint 1.7.12 and shfmt 3.13.1 from their official release artifacts with exact SHA-256 verification, then invoke both through fixed executable names supplied by the trusted step-local `PATH`; this removes the diff --git a/docs/doctoring/strix-nvidia-nim-not-found-fallback.md b/docs/doctoring/strix-nvidia-nim-not-found-fallback.md index b9a7459a59..46babc6d60 100644 --- a/docs/doctoring/strix-nvidia-nim-not-found-fallback.md +++ b/docs/doctoring/strix-nvidia-nim-not-found-fallback.md @@ -30,14 +30,14 @@ combining with an unrelated application `404` to spoof infrastructure fallback. Provider-side failure also remains a fail-closed incomplete scan until a distinct fallback produces complete evidence. -[Required run 32632284647](https://github.com/ContextualWisdomLab/.github/actions/runs/32632284647) -showed why the model-name boundary must be explicit. After NVIDIA capacity -failures, the workflow's `openai-direct/gpt-5.6-luna` routing alias reached -LiteLLM unchanged, so LiteLLM rejected it because `openai-direct` is not a -provider prefix. The shared gate now translates that workflow-only alias to -LiteLLM's documented `openai/gpt-5.6-luna` form before Strix starts. This is a -transport-normalization repair, not a model-selection change or a provider -failure classified as a successful scan. +[Required run 32640950204](https://github.com/ContextualWisdomLab/.github/actions/runs/32640950204) +proved that alias translation alone is not a complete cross-provider boundary: +the fallback also needs the trusted direct-OpenAI credential and must not retain +the NVIDIA API base. `ContextualWisdomLab/.github#1213` owns that complete +provider handoff together with the Azure unsupported-temperature classifier. +This change intentionally does not duplicate its partial model-only repair; +`#1213` must land first so the protected-base gate can produce authoritative +evidence for this pull request. Exhausted provider infrastructure remains fail-closed even when the trusted gate has classified every observed threshold finding as outside the pull @@ -58,8 +58,8 @@ Regression evidence proves that: context is not recognized; 5. model-catalog 404s enter cross-model fallback but never same-model retry; 6. the primary and first fallback are current NVIDIA hosted models; -7. the direct OpenAI workflow alias is translated to LiteLLM's `openai/` - provider prefix before invocation; +7. direct OpenAI cross-provider routing remains delegated to + `ContextualWisdomLab/.github#1213` rather than partially duplicated here; 8. provider exhaustion remains non-passing after unchanged baseline findings; 9. changed, unmapped, and changed-manifest findings also block after provider exhaustion; and @@ -76,9 +76,6 @@ Strix severity, changed-file attribution, or independent approval requirements. ## References -BerriAI. (n.d.). *LiteLLM documentation*. Retrieved August 23, 2026, from -https://docs.litellm.ai/ - Fielding, R., Nottingham, M., & Reschke, J. (2022). *HTTP semantics* (RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 4b10bc4c9a..3373730017 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -2452,10 +2452,6 @@ child_model_for_api_base() { printf 'openai/%s\n' "${model#openai_direct/}" return 0 ;; - openai-direct/*) - printf 'openai/%s\n' "${model#openai-direct/}" - return 0 - ;; esac printf '%s\n' "$model" diff --git a/tests/test_strix_nvidia_nim_not_found_fallback.py b/tests/test_strix_nvidia_nim_not_found_fallback.py index ceb87f1d60..11e2569c1f 100644 --- a/tests/test_strix_nvidia_nim_not_found_fallback.py +++ b/tests/test_strix_nvidia_nim_not_found_fallback.py @@ -213,29 +213,6 @@ def test_workflow_uses_available_free_first_nvidia_plan(self) -> None: )[0] self.assertNotIn(RETIRED_PRIMARY_MODEL, default_gate) - def test_direct_openai_fallback_alias_uses_litellm_provider_prefix(self) -> None: - """Translate the workflow alias before invoking LiteLLM through Strix.""" - - gate_source = STRIX_GATE.read_text(encoding="utf-8") - function_source = _function_block(gate_source, "child_model_for_api_base") - script = "\n".join( - ( - "set -euo pipefail", - "is_github_models_api_base() { return 1; }", - function_source, - 'child_model_for_api_base "$1" ""', - ) - ) - completed = subprocess.run( - ["bash", "-c", script, "strix-model", "openai-direct/gpt-5.6-luna"], - check=False, - capture_output=True, - text=True, - ) - - self.assertEqual(completed.returncode, 0, completed.stderr) - self.assertEqual(completed.stdout.strip(), "openai/gpt-5.6-luna") - def test_outer_workflow_requires_litellm_context_for_nvidia_404(self) -> None: """Reject provider-like target text in the outer neutralization gate.""" From b297581a3fa93440623c148c05e1b09579d3ed71 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 14:22:19 +0000 Subject: [PATCH 14/14] fix(scheduler+lint): case-fold repo identity; container/absolute-path shell detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Devin findings on PR #1231: 1. scripts/ci/pr_review_merge_scheduler.py: same_repository_head and compare_ref_for_pr_head compared headRepository.nameWithOwner to the configured target repo with exact case-sensitive equality, even though GitHub repository identity is case-insensitive and a sibling stale-run-cancellation path in this same file already case-folds. A same-repository PR whose GitHub-reported canonical name differed only in case from the configured target lost branch updates and automated merge eligibility. Fixed both to casefold consistently with the existing normalization idiom used elsewhere in the file. Added a regression covering same_repository_head, compare_ref_for_pr_head, and the end-to-end inspect_pr update_branch decision with a differently-cased headRepository. 2. scripts/ci/lint_github_workflows.rb: effective_shell ignored job-level `container:`, which defaults a step's shell to sh (not bash) per GitHub Actions semantics when no shell is configured anywhere in the resolution chain — so Bash-only syntax in a container job's default shell could pass validation as if it were bash. shellcheck_dialect also only recognized the bare names "bash"/"sh", skipping absolute-path custom shell templates (e.g. "/bin/bash --noprofile --norc -eo pipefail {0}", "/usr/bin/sh {0}") entirely rather than classifying them. Fixed by defaulting containerized jobs' shell to sh, and by classifying shellcheck_dialect off the executable's basename so absolute paths resolve the same as bare names. Added regression tests for both: a container job with no explicit shell (asserts shfmt is invoked with the posix dialect) and absolute-path bash/sh templates (asserts shfmt is invoked at all, with the correct dialect). All four changes verified against the pre-fix code: each new/extended test fails on the unpatched function and passes after the fix. Evidence: PYTHONPATH=. python3 -m pytest tests -q -> 1920 passed, 1 skipped, 21 subtests passed; coverage 100% statements/branches on scripts/ci; interrogate 100% docstrings; ruby -c clean; git diff --check clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --- scripts/ci/lint_github_workflows.rb | 18 +++-- scripts/ci/pr_review_merge_scheduler.py | 14 +++- tests/test_lint_github_workflows.py | 88 +++++++++++++++++++++++++ tests/test_pr_review_merge_scheduler.py | 17 +++++ 4 files changed, 131 insertions(+), 6 deletions(-) diff --git a/scripts/ci/lint_github_workflows.rb b/scripts/ci/lint_github_workflows.rb index a16b12fc10..c90ad536a1 100644 --- a/scripts/ci/lint_github_workflows.rb +++ b/scripts/ci/lint_github_workflows.rb @@ -59,17 +59,27 @@ def windows_runner?(job) end end +def containerized_job?(job) + !job["container"].nil? +end + def effective_shell(workflow, job, step) step["shell"] || job.dig("defaults", "run", "shell") || workflow.dig("defaults", "run", "shell") || - (windows_runner?(job) ? "pwsh" : "bash") + (windows_runner?(job) ? "pwsh" : (containerized_job?(job) ? "sh" : "bash")) end def shellcheck_dialect(shell) - return shell if ["bash", "sh"].include?(shell) - return "bash" if shell.start_with?("bash ") - return "sh" if shell.start_with?("sh ") + # A custom shell template names its executable as the first + # whitespace-delimited token (optionally followed by flags and a `{0}` + # script-path placeholder), and GitHub Actions accepts it as an absolute + # path (e.g. "/bin/bash --noprofile --norc -eo pipefail {0}" or + # "/usr/bin/sh {0}"), not just a bare "bash"/"sh" name. Resolve to the + # executable's basename so both forms classify identically. + executable = shell.to_s.split(" ", 2).first.to_s + name = File.basename(executable) + return name if ["bash", "sh"].include?(name) nil end diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 17fb9aaf16..5311cc41cb 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -964,7 +964,11 @@ def compare_ref_for_pr_head(repo: str, pr: dict[str, Any]) -> str: """Return the compare-API head ref for a PR branch.""" head_ref = pr.get("headRefName") or "HEAD" head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") - if not head_repo or head_repo == repo: + # GitHub repository identity is case-insensitive: casefold both sides so + # a same-repository head whose GitHub-reported canonical name differs + # only in case from the configured target is still treated as + # same-repository, matching same_repository_head below. + if not head_repo or head_repo.casefold() == repo.casefold(): return head_ref head_owner, _ = split_repo(head_repo) return f"{head_owner}:{head_ref}" @@ -1905,7 +1909,13 @@ def post_update_branch_followup( def same_repository_head(repo: str, pr: dict[str, Any]) -> bool: """Return whether the PR head branch belongs to the repository being scanned.""" head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") - return head_repo == repo + # GitHub repository identity is case-insensitive; casefold both sides so + # a same-repository PR whose GitHub-reported canonical name differs only + # in case from the configured target repo is not misclassified as + # cross-repository, matching the existing case-insensitive comparisons + # already used elsewhere in this file (e.g. the stale-run-cancellation + # gate and the central-repository dispatch-target check). + return bool(head_repo) and head_repo.casefold() == repo.casefold() def can_update_pr_head(repo: str, pr: dict[str, Any]) -> bool: diff --git a/tests/test_lint_github_workflows.py b/tests/test_lint_github_workflows.py index e1deb375e7..ea680f57e2 100644 --- a/tests/test_lint_github_workflows.py +++ b/tests/test_lint_github_workflows.py @@ -158,6 +158,94 @@ def test_linter_uses_actionlint_schema_and_bounded_shfmt_parser(tmp_path: Path) assert shfmt_records[1]["script"] == "set -e\necho ok\n" +def test_linter_treats_unshelled_container_job_step_as_posix_sh(tmp_path: Path) -> None: + """A container job with no explicit shell defaults to sh, not bash. + + GitHub Actions runs container-job steps under ``sh`` when no ``shell:`` + is configured anywhere in the resolution chain (step, job defaults, + workflow defaults) — unlike non-container Linux/macOS jobs, which + default to ``bash``. A linter that assumes bash here would validate + Bash-only syntax that the runner will actually execute as (potentially + broken) POSIX sh. + """ + + environment, capture_dir = _tool_environment(tmp_path) + workflow = tmp_path / "container-default-shell.yml" + workflow.write_text( + """name: container-default-shell +on: push +jobs: + containerized: + runs-on: ubuntu-24.04 + container: + image: debian:bookworm-slim + steps: + - name: No explicit shell + run: echo ok + bare: + runs-on: ubuntu-24.04 + steps: + - name: No explicit shell, no container + run: echo ok +""", + encoding="utf-8", + ) + + result = _run_linter(workflow, environment) + + shfmt_records = [ + json.loads(path.read_text(encoding="utf-8")) + for path in sorted(capture_dir.glob("shfmt-*.json")) + ] + assert result.returncode == 0, result.stderr + assert len(shfmt_records) == 2 + # Container job step: no explicit shell anywhere -> sh (posix). + assert shfmt_records[0]["args"] == ["-ln", "posix", "-tojson"] + # Non-container job step: no explicit shell anywhere -> bash, unchanged. + assert shfmt_records[1]["args"] == ["-ln", "bash", "-tojson"] + + +def test_linter_classifies_absolute_path_shell_templates(tmp_path: Path) -> None: + """Custom shell templates naming an absolute Bash/sh path are recognized. + + GitHub Actions accepts any executable, including an absolute path, as a + custom ``shell:`` template (optionally followed by flags and a ``{0}`` + script-path placeholder). A dialect matcher that only recognizes the + bare names "bash"/"sh" silently skips shell-syntax validation for such + steps instead of classifying them by dialect. + """ + + environment, capture_dir = _tool_environment(tmp_path) + workflow = tmp_path / "absolute-path-shell.yml" + workflow.write_text( + """name: absolute-path-shell +on: push +jobs: + verify: + runs-on: ubuntu-24.04 + steps: + - name: Absolute bash with flags + shell: '/bin/bash --noprofile --norc -eo pipefail {0}' + run: echo bash-ok + - name: Absolute sh with placeholder + shell: '/usr/bin/sh {0}' + run: echo sh-ok +""", + encoding="utf-8", + ) + + result = _run_linter(workflow, environment) + + shfmt_records = [ + json.loads(path.read_text(encoding="utf-8")) + for path in sorted(capture_dir.glob("shfmt-*.json")) + ] + assert result.returncode == 0, result.stderr + assert len(shfmt_records) == 2 + assert shfmt_records[0]["args"] == ["-ln", "bash", "-tojson"] + assert shfmt_records[1]["args"] == ["-ln", "posix", "-tojson"] + + def test_linter_preserves_lines_inside_multiline_expressions(tmp_path: Path) -> None: """Expression sanitizing keeps shfmt source and diagnostic lines aligned.""" diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index b593d40c80..61ca44dc77 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -3658,6 +3658,23 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert not sched.can_update_pr_head("owner/repo", external_behind) assert sched.can_update_pr_head("owner/repo", external_mutable) assert "same-repository head update permission" in sched.non_mutable_head_reason("owner/repo", behind) + # Regression: GitHub repository identity is case-insensitive. A PR whose + # GitHub-reported canonical headRepository differs from the configured + # target repo only by case must still be classified same-repository, so + # it keeps branch-update and merge eligibility instead of being + # misrouted onto the external/fork head path. + same_repo_different_case = make_pr( + mergeStateStatus="BEHIND", + headRepository={"nameWithOwner": "Owner/Repo"}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ) + assert sched.same_repository_head("owner/repo", same_repo_different_case) + assert sched.can_update_pr_head("owner/repo", same_repo_different_case) + assert sched.compare_ref_for_pr_head("owner/repo", same_repo_different_case) == "feature" + same_case_decision = inspect(same_repo_different_case) + assert same_case_decision.action == "update_branch" + assert called == [("owner/repo", 1, True)] + called.clear() behind_failed = make_pr( mergeStateStatus="BEHIND", reviews={"nodes": [opencode_review("APPROVED", "head")]},