From f8faecc2b7a41852ed9c89dc0af557e7e4dc6893 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:35:42 +0000 Subject: [PATCH 1/5] Bump the actions-deps group across 4 directories with 2 updates (#3150) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps the actions-deps group with 1 update in the /.github/actions/dotnet-publish-default directory: [actions/upload-artifact](https://github.com/actions/upload-artifact). Bumps the actions-deps group with 1 update in the /.github/actions/nuget-build-default directory: [actions/upload-artifact](https://github.com/actions/upload-artifact). Bumps the actions-deps group with 2 updates in the /.github/actions/pypi-build-default directory: [actions/upload-artifact](https://github.com/actions/upload-artifact) and [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv). Bumps the actions-deps group with 1 update in the /.github/actions/validate directory: [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv). Updates `actions/upload-artifact` from 7.0.1 to 7.0.2
Release notes

Sourced from actions/upload-artifact's releases.

v7.0.2

What's Changed

New Contributors

Full Changelog: https://github.com/actions/upload-artifact/compare/v7.0.1...v7.0.2

Commits

Updates `actions/upload-artifact` from 7.0.1 to 7.0.2
Release notes

Sourced from actions/upload-artifact's releases.

v7.0.2

What's Changed

New Contributors

Full Changelog: https://github.com/actions/upload-artifact/compare/v7.0.1...v7.0.2

Commits

Updates `actions/upload-artifact` from 7.0.1 to 7.0.2
Release notes

Sourced from actions/upload-artifact's releases.

v7.0.2

What's Changed

New Contributors

Full Changelog: https://github.com/actions/upload-artifact/compare/v7.0.1...v7.0.2

Commits

Updates `astral-sh/setup-uv` from 9.0.0 to 10.3.0
Release notes

Sourced from astral-sh/setup-uv's releases.

v10.3.0 🌈 Add Python architecture selection

Changes

This release adds the python-arch input to select a Python architecture independently of the Python version. With uv 0.13.0 or later, set python-arch: x86_64 to use x64 Python on Windows ARM64 runners.

It also includes embedded checksums for uv releases through 0.13.0.

🚀 Enhancements

🧰 Maintenance

⬆️ Dependency updates

v10.2.0 🌈 Disable automatic cache saves for merge queues

Changes

This release contains the known-checksum of the most recent uv releases and also disabled the uploading(saving) of the cache when in a merge queue since theses caches would almost never be used.

🚀 Enhancements

🧰 Maintenance

📚 Documentation

⬆️ Dependency updates

... (truncated)

Commits

Updates `astral-sh/setup-uv` from 8.1.0 to 10.3.0
Release notes

Sourced from astral-sh/setup-uv's releases.

v10.3.0 🌈 Add Python architecture selection

Changes

This release adds the python-arch input to select a Python architecture independently of the Python version. With uv 0.13.0 or later, set python-arch: x86_64 to use x64 Python on Windows ARM64 runners.

It also includes embedded checksums for uv releases through 0.13.0.

🚀 Enhancements

🧰 Maintenance

⬆️ Dependency updates

v10.2.0 🌈 Disable automatic cache saves for merge queues

Changes

This release contains the known-checksum of the most recent uv releases and also disabled the uploading(saving) of the cache when in a merge queue since theses caches would almost never be used.

🚀 Enhancements

🧰 Maintenance

📚 Documentation

⬆️ Dependency updates

... (truncated)

Commits

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore ` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore ` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore ` will remove the ignore condition of the specified dependency and ignore conditions
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/actions/dotnet-publish-default/action.yml | 2 +- .github/actions/nuget-build-default/action.yml | 4 ++-- .github/actions/pypi-build-default/action.yml | 4 ++-- .github/actions/validate/action.yml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/actions/dotnet-publish-default/action.yml b/.github/actions/dotnet-publish-default/action.yml index 655f4fd6f..917611296 100644 --- a/.github/actions/dotnet-publish-default/action.yml +++ b/.github/actions/dotnet-publish-default/action.yml @@ -102,7 +102,7 @@ runs: # GitHub-release asset, uploaded under the release-asset-- pattern the github-release job collects. - name: Upload release asset step if: ${{ inputs.smoke != 'true' }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: release-asset-${{ inputs.branch }}-dotnet-publish path: ${{ steps.zip.outputs.asset-path }} diff --git a/.github/actions/nuget-build-default/action.yml b/.github/actions/nuget-build-default/action.yml index 827b712c6..fc2673f5c 100644 --- a/.github/actions/nuget-build-default/action.yml +++ b/.github/actions/nuget-build-default/action.yml @@ -70,7 +70,7 @@ runs: # It runs inside the hub task's job, so an adopter's registry sees this hub in the job_workflow_ref claim and rejects the push. - name: Upload build artifacts step if: ${{ inputs.smoke != 'true' }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: nuget-build-${{ inputs.branch }} path: | @@ -89,7 +89,7 @@ runs: # GitHub-release asset, uploaded under the release-asset-- pattern the github-release job collects. - name: Upload release asset step if: ${{ inputs.smoke != 'true' }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: release-asset-${{ inputs.branch }}-nuget path: ${{ runner.temp }}/nuget.7z diff --git a/.github/actions/pypi-build-default/action.yml b/.github/actions/pypi-build-default/action.yml index ec95dd698..28467b689 100644 --- a/.github/actions/pypi-build-default/action.yml +++ b/.github/actions/pypi-build-default/action.yml @@ -36,7 +36,7 @@ runs: exit 1 - name: Setup uv step - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + uses: astral-sh/setup-uv@1c37ad07a6a961277cf70c0d37d6f313000f5884 # v10.3.0 with: version: ${{ inputs.uv-version }} enable-cache: true @@ -115,7 +115,7 @@ runs: # It runs inside the hub task's job, so an adopter's registry sees this hub in the job_workflow_ref claim and rejects the push. - name: Upload build artifacts step if: ${{ inputs.smoke != 'true' }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: pypi-build-${{ inputs.branch }} path: ${{ inputs.project-dir }}/dist/* diff --git a/.github/actions/validate/action.yml b/.github/actions/validate/action.yml index 453896175..a2db9eaa4 100644 --- a/.github/actions/validate/action.yml +++ b/.github/actions/validate/action.yml @@ -18,7 +18,7 @@ runs: python3 spec/validate.py - name: Setup uv step - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 + uses: astral-sh/setup-uv@1c37ad07a6a961277cf70c0d37d6f313000f5884 # v10.3.0 with: python-version: "3.13" From a040949784d829f1b80bb27df5e82bd0edcf3514 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 11:32:08 -0700 Subject: [PATCH 2/5] Reject a Dash-Prefixed Agents Name in Both Bootstrap Loaders (#3156) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #3129 `bootstrap.ps1 -Agents` and `bootstrap.sh --agents` now refuse any tool name that starts with a dash before the handoff to `install-agents`, with an error and a nonzero exit. Without it, `pwsh -File` binds `-All` or `-a` as a parameter of `install-agents.ps1` (and `--all` is read as an option by `install-agents.sh`), installing every tool. - bash: refused during argument parsing, per comma-separated name, before anything runs. - PowerShell: refused in `Invoke-AgentsInstall` over every split name, for ASCII hyphen and also U+2013, U+2014, U+2015, which PowerShell binds as a parameter prefix (autocorrected paste). The check runs after the tree fetch, as the empty-list check does. - Tests: one per loader in `tests/test_bootstrap.py`. Revert proof: with both loaders restored to develop, all subtests of both new tests fail; with the fix they pass. - Local review: two passes. Pass 1 found the typographic-dash bypass (fixed). Pass 2 found no code defect and one test gap (U+2015 case), added after the pass as a one-line test-only commit, so the recorded receipt covers that line unreviewed. Windows native verification (5.1 and 7) is left to the maintainer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5.5 --- host-setup/bootstrap.ps1 | 3 +++ host-setup/bootstrap.sh | 1 + tests/test_bootstrap.py | 17 +++++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/host-setup/bootstrap.ps1 b/host-setup/bootstrap.ps1 index 71b722e52..0a1d08789 100644 --- a/host-setup/bootstrap.ps1 +++ b/host-setup/bootstrap.ps1 @@ -488,6 +488,9 @@ function Invoke-Upgrade { # Installs only what is named, since the agent tools are never installed as a set nobody chose. function Invoke-AgentsInstall { if ($script:AGENT_NAMES.Count -eq 0) { die "-Agents takes the tools to install, separated by commas, or 'all'" } + foreach ($name in $script:AGENT_NAMES) { + if ($name -match '^[-\u2013\u2014\u2015]') { die "-Agents takes tool names, and '$name' starts with a dash" } + } if ($script:AGENT_NAMES -ccontains 'all') { if ($script:AGENT_NAMES.Count -gt 1) { die "-Agents takes 'all' alone, or a list of tool names" } Invoke-Tool -Tool 'install-agents.ps1' -Arguments '-Install', '-All' diff --git a/host-setup/bootstrap.sh b/host-setup/bootstrap.sh index 90f9796b0..911164b4b 100755 --- a/host-setup/bootstrap.sh +++ b/host-setup/bootstrap.sh @@ -409,6 +409,7 @@ parse_args() { IFS=',' read -r -a agent_fields <<<"$2" for agent_name in "${agent_fields[@]}"; do if [[ -n $agent_name ]]; then + [[ $agent_name != -* ]] || die "--agents takes tool names, and \"$agent_name\" starts with a dash" AGENTS+=("$agent_name") if [[ $agent_name == "all" ]]; then saw_all=true diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index ac1fe7f4b..8825d3526 100755 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -1795,6 +1795,15 @@ def test_agents_with_an_empty_list_is_refused_before_anything_runs(self) -> None self.assertEqual(self.steps(), []) self.assertEqual(list(self.data.iterdir()), []) + def test_agents_a_dash_prefixed_name_is_refused_before_anything_runs(self) -> None: + for given in ("--all", "-a", "herdr,--all", "-x,herdr"): + with self.subTest(given=given): + result = self.run_main(action="--agents", args=(given,)) + self.assertNotEqual(result.returncode, 0) + self.assertIn("starts with a dash", result.stderr) + self.assertEqual(self.steps(), []) + self.assertEqual(list(self.data.iterdir()), []) + def test_agents_with_no_list_is_refused_before_anything_runs(self) -> None: result = self.run_main(action="--agents") self.assertNotEqual(result.returncode, 0) @@ -1972,6 +1981,14 @@ def test_empty_names_are_dropped_from_the_list(self) -> None: self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(self.steps(), ["install-agents.ps1 -Install herdr cswap"], result.stdout) + def test_a_dash_prefixed_name_is_refused(self) -> None: + for given in ("-All", "-a", "herdr,-All", "\u2013All", "herdr,\u2014y", "\u2015All"): + with self.subTest(given=given): + result = self.run_agents([given]) + self.assertNotEqual(result.returncode, 0) + self.assertIn("starts with a dash", result.stderr) + self.assertEqual(self.steps(), []) + def test_an_empty_list_is_refused(self) -> None: result = self.run_agents([","]) self.assertNotEqual(result.returncode, 0) From f7a74a049c2a4349a267fc75ea602b1e7f44cec8 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 11:37:38 -0700 Subject: [PATCH 3/5] Test the Docker Task Workflow's Credential Check, Login, and Hook Inputs (#3154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #3148 ## Change Adds `tests/test_docker_task_workflow.py`, a stdlib-only structural test of `.github/workflows/build-docker-task.yml`. The job and step sets are read from the workflow by indentation, with no hard-coded indexes. Every job with a push-capable step (a step passing `push`, or a caller build hook) must: - run a credential check step (both secrets through env, both tested empty, `exit 1`, no `if` or `continue-on-error`) before its first push-capable step; - run the `docker/login-action` step fed both secrets, ungated, before its first push-capable step, and after the check; - forward `push`, `ref`, `branch`, and `platforms` to the caller hook, with `push`, `ref`, and `branch` wired to the same-named input. It also requires at least two pushing jobs, so a parse that silently loses a job fails. ## Mutation proof The test file holds these mutations in-memory against the real workflow text, and each asserts the specific violation. Also run once against a scratch copy: | Mutation | Result | | --- | --- | | drop login | `no login before the first push step` in docker-build-base and build-docker | | drop credential check | `no credential check before the first push step` in both | | login moved after the hook | `docker-build-base: no login before the first push step` | | login moved after build-push | `build-docker: no login before the first push step` | | check moved after login | `docker-build-base: login before the credential check` | | drop hook `push`, `ref`, `branch`, `platforms` (each) | `hook ... lacks ` | | `ref` wired to `inputs.branch` | `does not forward inputs.ref` | | half the credential test, `continue-on-error`, `if` gate, commented-out password | caught | ## Local review Three passes. Rounds 1 and 2 findings were fixed (move-helper newline glue, substring false passes on comments and gating, a mutation test that gated the wrong job's identical step, missing check-after-login and half-check mutations, hook input values). The third pass raised five findings, left unfixed per the two-round budget and handed here as known limits of an indentation-based reader: - `||` joining the two empty tests is not checked (`&&` would pass). - The login step is not checked for `username`/`password` wiring or a non-Hub `registry:`. - Compact YAML (`- name:` at the same indent as `steps:`) and flow-style `with: {push: true}` are not parsed, so a new job written that way is unjudged beyond the two-job floor. - Earlier, accepted: a `run:` step that pushes is not push-capable, and a step whose first key is not `name:` is unsupported. Verified: ruff format and check, pyright, `prose_lint`, `repo_gate --check eol`, `eol-coverage`, `spec/validate.py`, 12 tests pass. Co-authored in a Claude Code session. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5.5 --- tests/test_docker_task_workflow.py | 299 +++++++++++++++++++++++++++++ 1 file changed, 299 insertions(+) create mode 100755 tests/test_docker_task_workflow.py diff --git a/tests/test_docker_task_workflow.py b/tests/test_docker_task_workflow.py new file mode 100755 index 000000000..545a6021c --- /dev/null +++ b/tests/test_docker_task_workflow.py @@ -0,0 +1,299 @@ +#!/usr/bin/env python3 +"""Pin the Docker task workflow's login-before-push order and its hook inputs. + +Every job that can push to Docker Hub must check the credentials and log in before the first step +that pushes, the caller's hook step included, and the hook step must receive the inputs it builds +from. The job and step sets are read from the workflow, so a reordered, renamed, or added job is +judged by the same rule rather than by a hard-coded index. +""" + +from __future__ import annotations + +import re +import unittest +from pathlib import Path +from typing import NamedTuple + +REPO = Path(__file__).resolve().parents[1] +WORKFLOW = REPO / ".github/workflows/build-docker-task.yml" + +LOGIN_ACTION = "docker/login-action@" +HOOK_INPUTS = ("push", "ref", "branch", "platforms") +CREDENTIAL_NAMES = ("DOCKER_HUB_USERNAME", "DOCKER_HUB_ACCESS_TOKEN") + + +class Step(NamedTuple): + """One step: its name, `uses` value, `with` keys, and raw text.""" + + name: str + uses: str + with_keys: tuple[str, ...] + text: str + + +def job_texts(workflow: str) -> dict[str, str]: + """Each job's text keyed by job id, read by indentation, since the suite runs on the standard library alone.""" + lines = workflow.splitlines() + start = next(i for i, line in enumerate(lines) if re.fullmatch(r"jobs:\s*(#.*)?", line)) + jobs: dict[str, list[str]] = {} + current: list[str] | None = None + for line in lines[start + 1 :]: + header = re.fullmatch(r" ([A-Za-z0-9_-]+):\s*(#.*)?", line) + if header: + current = jobs.setdefault(header.group(1), []) + elif line.strip() and not line.startswith(" "): + break + if current is not None: + current.append(line) + return {job: "\n".join(body) for job, body in jobs.items()} + + +def job_steps(job: str) -> list[Step]: + """The job's steps in order, each bounded to its own sequence item.""" + items: list[list[str]] = [] + in_steps = False + for line in job.splitlines(): + if re.fullmatch(r" steps:\s*(#.*)?", line): + in_steps = True + elif in_steps and line.startswith(" - "): + items.append([line]) + elif in_steps and items and (not line.strip() or line.startswith(" ")): + items[-1].append(line) + elif in_steps and line.strip() and not line.lstrip().startswith("#"): + break + steps = [] + for item in items: + text = "\n".join(item) + name = re.search(r"(?m)^ - name: (.+)$", text) + uses = re.search(r"(?m)^ uses: [\"']?([^\s\"']+)", text) + steps.append( + Step( + name.group(1) if name else "", + uses.group(1) if uses else "", + with_keys(item), + text, + ) + ) + return steps + + +def with_keys(item: list[str]) -> tuple[str, ...]: + """The keys directly under the step's `with:`, ignoring comments.""" + keys: list[str] = [] + in_with = False + for line in item: + if re.fullmatch(r" with:\s*(#.*)?", line): + in_with = True + elif in_with: + key = re.match(r" ([A-Za-z0-9_-]+):", line) + if key: + keys.append(key.group(1)) + elif line.strip() and not line.startswith(" "): + break + return tuple(keys) + + +def ungated(step: Step) -> bool: + """The step has no `if` or `continue-on-error`, so it always runs and a failure stops the job.""" + return not re.search(r"(?m)^ (?:if|continue-on-error):", step.text) + + +def is_credential_check(step: Step) -> bool: + """A run step reading both Docker Hub secrets through env and testing them empty.""" + return ( + not step.uses + and " run: |" in step.text + and all(f"{name}: ${{{{ secrets.{name} }}}}" in step.text for name in CREDENTIAL_NAMES) + and all(f'-z "${name}"' in step.text for name in CREDENTIAL_NAMES) + and "exit 1" in step.text + and ungated(step) + ) + + +def is_login(step: Step) -> bool: + """The Docker login action fed both secrets.""" + return ( + step.uses.startswith(LOGIN_ACTION) + and all(f"secrets.{name}" in step.text for name in CREDENTIAL_NAMES) + and ungated(step) + ) + + +def can_push(step: Step) -> bool: + """A step that can push: one passing `push`, or a caller build hook even if its push is lost.""" + is_build_hook = step.uses.startswith("./.github/actions/") and "build" in step.uses + return "push" in step.with_keys or is_build_hook + + +def violations(workflow: str) -> list[str]: + """Every way the workflow breaks the login-before-push order or the hook inputs.""" + workflow = "\n".join( + line for line in workflow.splitlines() if not line.lstrip().startswith("#") + ) + found: list[str] = [] + pushing = 0 + for job_id, body in job_texts(workflow).items(): + steps = job_steps(body) + pushes = [i for i, step in enumerate(steps) if can_push(step)] + if not pushes: + continue + pushing += 1 + first_push = pushes[0] + checks = [i for i, step in enumerate(steps) if is_credential_check(step)] + logins = [i for i, step in enumerate(steps) if is_login(step)] + if not checks or checks[0] > first_push: + found.append(f"{job_id}: no credential check before the first push step") + if not logins or logins[0] > first_push: + found.append(f"{job_id}: no login before the first push step") + if checks and logins and checks[0] > logins[0]: + found.append(f"{job_id}: login before the credential check") + for step in steps: + if can_push(step) and step.uses.startswith("./.github/actions/"): + missing = [key for key in HOOK_INPUTS if key not in step.with_keys] + if missing: + found.append(f"{job_id}: hook {step.uses} lacks {', '.join(missing)}") + for key in ("push", "ref", "branch"): + if f" {key}: ${{{{ inputs.{key} }}}}" not in step.text: + found.append(f"{job_id}: hook {step.uses} does not forward inputs.{key}") + if pushing < 2: + found.append(f"expected at least two pushing jobs, found {pushing}") + return found + + +STEP_END = r"(?=^ (?:- |#)|^ \S|\Z)" + + +def step_text(body: str, name: str) -> str: + """The raw text of the one step of that name in a job body.""" + matches = re.findall(rf"(?ms)^ - name: {re.escape(name)}\n.*?{STEP_END}", body) + assert len(matches) == 1, name + return matches[0] + + +def drop_step(workflow: str, name: str) -> str: + """The workflow text without every step of that name.""" + return re.sub(rf"(?ms)^ - name: {re.escape(name)}\n.*?{STEP_END}", "", workflow) + + +def move_step_after(workflow: str, job: str, name: str, anchor: str) -> str: + """Move the step `name` to directly after the step `anchor`, both within one job.""" + body = job_texts(workflow)[job] + moved = step_text(body, name) + cut = body.replace(moved, "", 1) + anchor_text = step_text(cut, anchor) + end = cut.index(anchor_text) + len(anchor_text) + head = cut[:end] if cut[:end].endswith("\n") else cut[:end] + "\n" + return workflow.replace(body, head + moved + cut[end:], 1) + + +class DockerTaskWorkflowCase(unittest.TestCase): + """The checked-in workflow passes, and each mutation of it is caught.""" + + workflow = WORKFLOW.read_text(encoding="utf-8") + + def test_workflow_is_clean(self) -> None: + self.assertEqual([], violations(self.workflow)) + + def test_both_known_jobs_are_judged(self) -> None: + pushing = { + job_id + for job_id, body in job_texts(self.workflow).items() + if any(can_push(step) for step in job_steps(body)) + } + self.assertTrue({"build-docker", "docker-build-base"} <= pushing) + + def test_dropped_login_is_caught(self) -> None: + mutated = drop_step(self.workflow, "Login to Docker Hub step") + self.assertNotEqual(self.workflow, mutated) + self.assertIn("docker-build-base: no login before the first push step", violations(mutated)) + self.assertIn("build-docker: no login before the first push step", violations(mutated)) + + def test_dropped_credential_check_is_caught(self) -> None: + mutated = drop_step(self.workflow, "Require Docker Hub credentials step") + self.assertNotEqual(self.workflow, mutated) + found = violations(mutated) + self.assertIn("docker-build-base: no credential check before the first push step", found) + self.assertIn("build-docker: no credential check before the first push step", found) + + def test_login_moved_after_the_hook_is_caught(self) -> None: + hook = "Run caller docker-build-base hook step" + mutated = move_step_after( + self.workflow, "docker-build-base", "Login to Docker Hub step", hook + ) + self.assertNotEqual(self.workflow, mutated) + self.assertIn("docker-build-base: no login before the first push step", violations(mutated)) + + def test_login_moved_after_the_build_is_caught(self) -> None: + build = "Docker build and push step" + mutated = move_step_after(self.workflow, "build-docker", "Login to Docker Hub step", build) + self.assertNotEqual(self.workflow, mutated) + self.assertIn("build-docker: no login before the first push step", violations(mutated)) + + def test_check_moved_after_the_login_is_caught(self) -> None: + mutated = move_step_after( + self.workflow, + "docker-build-base", + "Require Docker Hub credentials step", + "Login to Docker Hub step", + ) + self.assertNotEqual(self.workflow, mutated) + self.assertIn("docker-build-base: login before the credential check", violations(mutated)) + + def test_gated_check_and_login_are_caught(self) -> None: + for name, flag in ( + ("Require Docker Hub credentials step", "continue-on-error: true"), + ("Login to Docker Hub step", "if: ${{ inputs.push }}"), + ): + with self.subTest(name=name): + body = job_texts(self.workflow)["build-docker"] + step = step_text(body, name) + gated = step.replace("\n ", f"\n {flag}\n ", 1) + mutated = self.workflow.replace(body, body.replace(step, gated, 1), 1) + self.assertNotEqual(self.workflow, mutated) + found = violations(mutated) + self.assertTrue(any(v.startswith("build-docker: no ") for v in found)) + + def test_half_a_credential_check_is_caught(self) -> None: + mutated = self.workflow.replace(' || -z "$DOCKER_HUB_ACCESS_TOKEN"', "", 1) + self.assertNotEqual(self.workflow, mutated) + self.assertIn( + "docker-build-base: no credential check before the first push step", violations(mutated) + ) + + def test_miswired_hook_input_is_caught(self) -> None: + mutated = self.workflow.replace( + " ref: ${{ inputs.ref }}\n branch:", + " ref: ${{ inputs.branch }}\n branch:", + 1, + ) + self.assertNotEqual(self.workflow, mutated) + self.assertIn( + "docker-build-base: hook ./.github/actions/docker-build-base does not forward inputs.ref", + violations(mutated), + ) + + def test_commented_out_secret_is_caught(self) -> None: + mutated = self.workflow.replace( + " password: ${{ secrets.DOCKER_HUB_ACCESS_TOKEN }}", + " # password: ${{ secrets.DOCKER_HUB_ACCESS_TOKEN }}", + ) + self.assertNotEqual(self.workflow, mutated) + self.assertIn("no login before the first push step", "\n".join(violations(mutated))) + + def test_each_dropped_hook_input_is_caught(self) -> None: + body = job_texts(self.workflow)["docker-build-base"] + hook = step_text(body, "Run caller docker-build-base hook step") + for key in HOOK_INPUTS: + with self.subTest(key=key): + lines = [ + line + for line in hook.splitlines(keepends=True) + if not line.startswith(f" {key}:") + ] + mutated = self.workflow.replace(hook, "".join(lines), 1) + self.assertNotEqual(self.workflow, mutated) + self.assertTrue(any(f"lacks {key}" in v for v in violations(mutated))) + + +if __name__ == "__main__": + unittest.main() From 7a58899973000730a0bb93ff195794e4eaa74eaa Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 11:38:14 -0700 Subject: [PATCH 4/5] Let a Manual Dispatch Bypass the Smoke Build's Paths Filter (#3153) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #3149 The release-with-smoke caller stub gated `smoke-build` only on the paths-filter output, so a `workflow_dispatch` run (no changed paths) always skipped the build. Per the maintainer decision on the issue, `smoke-build` now runs when `github.event_name == 'workflow_dispatch' || needs.changes.outputs.release == 'true'`. The `workflow_dispatch:` trigger stays. The change is in `catalog/snippets/workflows/test-pull-request-smoke.yml` and the matching example in `docs/reusable-workflows.md`, which stay byte-identical. Verification - `tests/test_release_guards.py::ReleaseGuardCase::test_smoke_caller_stub_is_its_snippet_byte_for_byte` passes. Revert proof: reverting only the docs copy makes it fail. - eol, eol-coverage, spec/validate.py, prose_lint, markdownlint and editorconfig-checker are clean. actionlint is not installed locally. - Local strict review: 0 findings. The aggregator still treats skipped as pass, and the dispatch `branch` input resolves via `ref_name`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5.5 --- catalog/snippets/workflows/test-pull-request-smoke.yml | 2 +- docs/reusable-workflows.md | 2 +- tests/test_release_guards.py | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/catalog/snippets/workflows/test-pull-request-smoke.yml b/catalog/snippets/workflows/test-pull-request-smoke.yml index f4c8c0d08..0a5c47f1e 100644 --- a/catalog/snippets/workflows/test-pull-request-smoke.yml +++ b/catalog/snippets/workflows/test-pull-request-smoke.yml @@ -51,7 +51,7 @@ jobs: smoke-build: name: Smoke build job needs: [changes] - if: needs.changes.outputs.release == 'true' + if: github.event_name == 'workflow_dispatch' || needs.changes.outputs.release == 'true' uses: ptr727/ProjectTemplate/.github/workflows/build-release-task.yml@6c88850128629bb449fd73c7fbf82968c6cd1557 # 2.0.587 permissions: contents: read diff --git a/docs/reusable-workflows.md b/docs/reusable-workflows.md index b2b37669d..9b7dab5a4 100644 --- a/docs/reusable-workflows.md +++ b/docs/reusable-workflows.md @@ -355,7 +355,7 @@ jobs: smoke-build: name: Smoke build job needs: [changes] - if: needs.changes.outputs.release == 'true' + if: github.event_name == 'workflow_dispatch' || needs.changes.outputs.release == 'true' uses: ptr727/ProjectTemplate/.github/workflows/build-release-task.yml@6c88850128629bb449fd73c7fbf82968c6cd1557 # 2.0.587 permissions: contents: read diff --git a/tests/test_release_guards.py b/tests/test_release_guards.py index fc66f1184..79c30d7c5 100755 --- a/tests/test_release_guards.py +++ b/tests/test_release_guards.py @@ -722,6 +722,10 @@ def test_smoke_caller_stub_is_its_snippet_byte_for_byte(self) -> None: blocks = yaml_blocks((REPO / "docs/reusable-workflows.md").read_text(encoding="utf-8")) self.assertIn(stub, blocks) self.assertIn("smoke: true", stub) + self.assertIn( + " if: github.event_name == 'workflow_dispatch' || needs.changes.outputs.release == 'true'\n", + stub, + ) def test_release_asset_hook_upload_stays_with_the_task(self) -> None: workflow = (REPO / ".github/workflows/build-release-task.yml").read_text(encoding="utf-8") From a152f4790ca3954d5a628f2d1456d1ae2a7c4150 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 16 Oct 2026 13:03:18 -0700 Subject: [PATCH 5/5] Read the gh Write Targets the Write Guard Still Missed (#3162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary The write guard's cross-owner check (rule 3) read a write's target only from a repo flag, a `repos/` API path, and a `GH_REPO` set in the command text. gh reads its target from more places than that. This change reads each of them, building on the command and subcommand reader #3135 added. Every new reading only adds a target, so nothing develop denies is allowed. - **An inherited `GH_REPO` (#3147).** The value in the environment the session was launched with now reaches every gh word, wrapper bodies and substitutions included. A command-text prefix or a repo flag still overrides it, as gh reads it. The hook reads the same environment `GH_WRITE_GUARD_ALLOW` is read from. A value a Bash call gets another way, such as from a shell profile, is not seen, and the README says so. - **A run-time word before gh's command, and a bare export (#3136).** `gh "$R" pr comment 5` is read as a comment whose run-time word may be a repo flag, so it fails closed, as `gh pr "$R" comment` does. `GH_REPO=x; export GH_REPO` reaches gh. So does a plain assignment once GH_REPO is exported, and an `export` argument past a bare name. - **A gh write behind a run-time command word (#3131).** `$G`, `"$G"`, `${G}`, and `"$(command -v gh)"` before a literal gh write are read as gh, for rules 1, 2, 3, and 5. - **Positional repositories and missing write subcommands (#3133).** A pull request or issue URL argument names its repository. A `gh repo` write's repository argument, `gh issue transfer`'s destination, and `gh issue develop --branch-repo` are read too, in gh's `[HOST/]OWNER/REPO`, https, ssh, and scp forms. The table gains `pr revert`/`update-branch`, `issue develop`, `release delete-asset`, `repo unarchive`/`sync`, `label clone`, `workflow run`/`enable`/`disable`, `run rerun`/`cancel`/`delete`, `cache delete`, `repo deploy-key add`/`delete`, and `repo autolink create`/`delete`. - **`gh api` write targets outside `repos/` (#3132).** `orgs/`, `users/`, `user/starred//`, `user/following|blocks/`, and `user/memberships/orgs/` name an owner. An api write whose endpoint opens with a run-time word, or whose owner segment is one, fails closed. - **Wrapper-scoped `GH_REPO` (#3144, under-deny side).** Each wrapper's own reaching value is now read for its own bodies, nested ones inheriting. So is a quoted `"$(...)"` substitution's. Develop allowed `GH_REPO=stranger/example bash -c 'gh issue comment 5 -b hi'; GH_REPO=example/repo bash -c 'gh pr view 5'`. The old latest-value reading is kept beside it, so the issue's own over-deny example stays denied. - **#3145.** The merge deny message now names a quoted run-time word where gh itself goes. `gh api`'s older `:owner`/`:repo` placeholders take `GH_REPO` (confirmed live: gh 2.102 still fills them). A three-part value is read by its middle owner as well, which closes `--repo example/stranger/x`. The first-part reading is kept, so `GH_REPO=github.com/example/repo` stays denied. The maintainer's rule on #3079 holds throughout: a run-time repository value fails closed only when its owner is run-time, so `gh repo create "example/$NAME"` and `orgs/example/$T` are compared by their literal owner. ## Scope Choices - An environment `GH_REPO` is replaced only by a prefix, on the gh word or on its wrapper. A command-text export can sit where it never runs, in a subshell, an untaken branch, a function, or a backgrounded or piped command, so the environment value is read beside every export. That denies `export GH_REPO=example/repo; gh pr comment 5` in a session whose environment holds a foreign `GH_REPO`, a deliberate over-deny. A prefix, `GH_REPO=example/repo gh pr comment 5`, or `-R` is the allowed spelling, and the Claude README now says so. - A wholly run-time pull request or issue selector, as in `gh pr comment "$PR"`, is left allowed. Whether it should fail closed is #3157. ## Develop-Versus-Branch Differential The corpus is every command in develop's self-test tables (1255 commands) plus 165 constructed commands, each tagged with the issue it exercises or as a control. Each was run under both guards with origin `example/repo` and again with a second owner, 2840 runs. - **Flips:** 189, every one allow to deny, and **new allows: 0**. - **Every flip has an issue:** #3147 (38), #3136 (27), #3131 (23), #3133 (64), #3132 (27), #3145 (7), #3144 (3). Those counts include the 11 flips tagged control. All 11 are under the second owner, where `example/...` is itself a foreign owner, so each is the same shape as its issue's case: one #3136, one #3131, six #3133, and three #3132. - **Unexplained flips:** none. No case from develop's own tables flipped. - **Fuzzing:** 42,000 random constructed commands across the branch's heads, 6,000 of them on the final one, plus about 100,000 to 160,000 per pass from the local reviewer. No new allow, and no exception from `classify`. An exception there would let the hook fail open. ## Tests and Revert Proof - `_GH_TARGET_CASES` adds 82 self-tests, run with the environment each names, origin `example/repo`. They cover every closed gap and a control for each. - **Revert proof:** the 82 cases were run against develop's guard, that is, with the fix reverted. All 56 deny cases fail there and all 26 allow controls pass, so each test fails without its fix. - `--selftest` passes with 1413 cases. `python3 -m unittest discover -s tests` passes 2873 tests, and `test_install.py` passes 162. - ruff check and format, mypy, `prose_lint.py --diff origin/develop`, `repo_gate.py --check eol` and `eol-coverage`, `spec/validate.py`, markdownlint, editorconfig-checker, and cspell are all clean. ## Local Review Five full passes at `frontier`, plus a receipt pass over the last README sentence. Round one raised 8 findings. These were fixed: - the shell state now reaches a gh word behind a launcher or `find -exec` - a substitution takes the state at the word opening it - ssh and scp repository forms are read - rule 2 now covers `$G api graphql` Rounds two through four were all about one question, when a command-text export may replace an environment `GH_REPO`. Each test that let one replace it was missing a case: a subshell, a backgrounded or piped export, a `case`, an if or loop body, or a quoted separator. The last test was also quadratic on long commands. That logic is deleted rather than patched again, so only a prefix replaces the value, per the scope choice above. A 26k-character command classifies in 0.46s, as on develop. Round two also found a gh word that is only an `echo` argument taking `GH_REPO`, and that is fixed. Round five found no code defect and a README gap, which is filled. These were filed: - the run-time selector, as #3157 - `--org`/`--owner` flags, issue-reference flag values, `repositories/` paths, `eval` payloads, dot-segments, and `declare -x`, as #3158 These were declined: - merging `_export_words` into the git-side export reader, which would change git-rule verdicts outside these issues - the ungated `sentence-length` check, which has about 175 existing hits in these files - a Windows env-name case item, which is unlikely Closes #3147 Closes #3136 Closes #3131 Closes #3133 Closes #3132 Refs #3144, whose over-deny example waits for the over-denial lane Refs #3145, whose item 3 over-deny (`GH_REPO=github.com/example/repo` denied) waits for the same lane 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- host-setup/agent-safety/README.md | 15 +- host-setup/agent-safety/claude/README.md | 2 +- .../agent-safety/claude/gh-write-guard.py | 847 ++++++++++++++++-- 3 files changed, 799 insertions(+), 65 deletions(-) diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index 622c79231..542cffa40 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -54,10 +54,19 @@ text says, because the harm it covers was never in the text. precision-over-recall stance every requirement but 4 takes. A write is recognized by its command and subcommand as gh's own parser finds them, past the flags before each. So a repo flag before the command or between `pr` and its subcommand still names the target. The target is also the - `R` in a short-option cluster such as `-dR`, and a `GH_REPO` value that reaches the gh word. One + `R` in a short-option cluster such as `-dR`, and a `GH_REPO` value that reaches the gh word, + the one in the environment the session was launched with included. A repository named by a + write's positional argument is one too, as in a pull request or issue URL, and so is the + repository `gh repo edit` or `gh issue transfer` takes. On a `gh api` write, an owner named by an + `orgs/`, `users/`, or `user/...` path counts, as in `orgs//...` or `user/starred//`. A + `[HOST/]OWNER/REPO` value is read by its middle owner as well as by its first part. A gh write + run through a run-time command word, as in `G=gh; $G pr comment ...`, is read as one. One exception departs from that stance. A write whose repository owner is a run-time word fails - closed, since it leaves nothing to compare. That covers `--repo "$T"`, `GH_REPO="$T"`, and - `repos/$O/...`, while `--repo "example/$R"` names its owner and is compared as usual. + closed, since it leaves nothing to compare. That covers `--repo "$T"`, `GH_REPO="$T"`, + `repos/$O/...`, a run-time repository positional, and a `gh api` write whose endpoint opens + with a run-time word. It also covers a run-time word before gh's command that may hold a repo + flag, as in `gh "$R" pr comment`. A `--repo "example/$R"` names its owner and is compared as + usual. 4. **A git operation that would only succeed by bypassing an active branch rule is denied**: a direct push to a branch whose rules require a pull request, a force-push where history is protected, a delete where deletion is blocked, or an explicit-bypass flag (`--admin` on a merge, diff --git a/host-setup/agent-safety/claude/README.md b/host-setup/agent-safety/claude/README.md index b935436ee..0fff34805 100644 --- a/host-setup/agent-safety/claude/README.md +++ b/host-setup/agent-safety/claude/README.md @@ -155,7 +155,7 @@ Every other key in the file is left as it stands, `permissions.allow` and the re - **Per-machine.** `~/.claude/` does not travel, so run the installer on each box. This is the rollout that [#365][issue-365] tracks. - **Precision over recall for the write footguns.** The hook denies the specific dangerous write shapes with high confidence rather than gating every write, so it is written to avoid blocking legitimate work -- a known false-positive shape is a parser defect to fix, not an accepted cost. The exception is a write whose repository owner is a run-time word, denied because it leaves nothing to compare. A shape it does not catch still falls under the behavioral rules. - **The branch-bypass rule fails closed.** Unlike the write-footgun rules, a push to `main`/`master`/`develop` is denied even when its rules cannot be determined (the API is unreachable, or the checkout's origin cannot be resolved to query them), because the harm there is a silent success under the maintainer's admin bypass. The rule reads each branch's live rules, so it adapts to every repo (a code-style `develop` denies, a config-style `develop` allows) with no per-repo configuration, and hands the exact command to the maintainer to run when a bypass is genuinely intended. -- **Opaque targets are unseen.** The hook cannot see the repository behind a GraphQL node id, which is exactly why rule 2 blocks a *literal* id at all, since a captured `$variable` is trusted. Likewise, the cross-origin check only runs when an `origin` can be resolved and the write names an explicit `-R`/`repos//`/`GH_REPO` target. A write from a non-git directory, or one whose target is only a node id, is evaluated by rules 1 and 2 alone. +- **Opaque targets are unseen.** The hook cannot see the repository behind a GraphQL node id, which is exactly why rule 2 blocks a *literal* id at all, since a captured `$variable` is trusted. Likewise, the cross-origin check only runs when an `origin` can be resolved and the write names an explicit target, such as `-R`, `repos//`, a `GH_REPO` value, or a pull request URL. A write from a non-git directory, or one whose target is only a node id, is evaluated by rules 1 and 2 alone. The hook reads an inherited `GH_REPO` from its own environment, the one the session was launched with. A value a Bash tool call gets some other way, such as from a shell profile it sources, is not seen. An inherited value is read beside any `export` in the command, since the hook cannot tell whether that export runs. To write elsewhere, override it with a prefix, as in `GH_REPO=/ gh ...`, or with `-R`. - **A write or a wait inside a script file is unseen, so never put either in a script to run it.** The hook reads the command the agent runs, which for `bash deploy.sh` is that one string. A `git push` or a `gh` mutation inside the script therefore reaches the server unparsed. An unbounded wait inside that same script is equally unseen, and worse in one way. A denied write fails loudly. A wait the guard never saw runs until something ends it, which is one of the leaks the SessionEnd sweep exists to find afterwards. This is the widest hole in the kit, and an agent opens it by accident. Collecting fifteen repetitive pushes into a loop looks like tidiness, and so does moving a poll into a script to keep a tool call short. Neither looks like disabling a guard. Issue each write and each wait as its own command. A script that only reads, computes, or prepares local commits is fine, because the boundary is the write and the wait rather than the script. - **Not a credential control.** A fine-grained PAT limited to owned repositories is a separate, stronger structural guard (a hard `403` on any non-owned repo) and is left to per-machine credential setup, out of this kit. - **Rule 6's ref-checkout and `--ff-only` exemptions are deliberate, not a gap.** The #1073 incident's own literal commands (an unforced `git checkout main`, then `git pull --ff-only`) are exactly this shape, so this rule does not deny the incident's own commands specifically. Denying them would also break the documented base-clone cleanup step in the `repo-worktree` skill, which needs precisely those two. The concurrent-access hazard those commands still carry (switching HEAD, or fast-forwarding, in a checkout another task might be relying on) is not decidable from the command text alone, so it stays the prose rule's job. A non-force flag alongside the ref, such as `--detach`, is exempt too, not only a strictly zero-flags invocation -- git's own overwrite-refusal on a dirty tracked file is unaffected by it, verified live. diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index 956f5d7ec..5b5c92db7 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -17,8 +17,9 @@ 1. a state-changing gh call whose output is discarded or forced to success (>/dev/null, 2>/dev/null, &>/dev/null, || true, || :, || echo) 2. a GraphQL mutation passing a literal GitHub node id (PRRT_/PR_/BOT_/...) instead of a $variable - 3. a gh write with an explicit -R/--repo/repos// target under an owner other than the - checkout origin's, unless the maintainer granted that target in GH_WRITE_GUARD_ALLOW. Sibling + 3. a gh write with an explicit target, such as -R/--repo, repos//, GH_REPO, or a pull + request URL, under an owner other than the checkout origin's, unless the maintainer granted + that target in GH_WRITE_GUARD_ALLOW. Sibling repositories under the same owner are allowed, since the harm this guards is reaching a stranger's repository, not working across your own fleet in one session. 4. a git operation that would only land by bypassing an active branch rule: a direct push to a branch @@ -110,15 +111,24 @@ "pr": ( *("create", "comment", "close", "merge", "edit"), *("review", "reopen", "ready", "lock", "unlock"), + *("revert", "update-branch"), ), "issue": ( *("create", "comment", "close", "edit", "reopen", "delete"), - *("lock", "unlock", "pin", "unpin", "transfer"), + *("lock", "unlock", "pin", "unpin", "transfer", "develop"), ), - "release": ("create", "edit", "delete", "upload"), - "repo": ("create", "delete", "edit", "rename", "archive"), - **{g: ("create", "delete", "edit", "set") for g in ("label", "secret", "variable", "ruleset")}, + "release": ("create", "edit", "delete", "upload", "delete-asset"), + "repo": ("create", "delete", "edit", "rename", "archive", "unarchive", "sync"), + **{g: ("create", "delete", "edit", "set") for g in ("secret", "variable", "ruleset")}, + "label": ("create", "delete", "edit", "set", "clone"), "gist": ("create", "edit", "delete"), + "workflow": ("run", "enable", "disable"), + "run": ("rerun", "cancel", "delete"), + "cache": ("delete",), +} +_GH_WRITE_SUBSUBS = { + ("repo", "deploy-key"): ("add", "delete"), + ("repo", "autolink"): ("create", "delete"), } # The gh subcommands that mutate. # A path-qualified or `.exe`-suffixed `gh` still starts a shell word this matches, the same recognition `_is_gh_exe` gives it for argv-position parsing. @@ -126,6 +136,8 @@ _GH_WRITE_SUB = re.compile( r"\bgh(?:\.exe)?\s+(?:" + "|".join(rf"{g}\s+(?:{'|'.join(subs)})" for g, subs in _GH_WRITE_SUBS.items()) + + "|" + + "|".join(rf"{g}\s+{s}\s+(?:{'|'.join(subs)})" for (g, s), subs in _GH_WRITE_SUBSUBS.items()) + r")\b" ) _GRAPHQL = re.compile(r"\bgh(?:\.exe)?\s+api\b.*\bgraphql\b", re.DOTALL) @@ -233,7 +245,11 @@ def _gh_argv_is_write(args): if not args: return False if args[0] != "api": - return _gh_path_writes(_gh_path(args)) + return ( + _gh_path_writes(_gh_path(args)) + or _gh_path3_writes(args) + or _gh_write_past_run_time(args) is not None + ) path = _gh_api_path(args) if path == "graphql": if _gh_has_input(args): @@ -246,23 +262,23 @@ def _gh_argv_is_write(args): _GH_PATH_BOOL_FLAGS = frozenset({"--help", "--version"}) -def _gh_path(args): +def _gh_path(args, limit=2): """gh's command and subcommand words in an argv, at most two, read the way cobra finds them. Cobra reads past each flag before them. A flag word with no `=` that is `--long` or one letter takes the next word as its value, unless it is a known boolean. An unknown flag is no boolean to it, so `gh pr --squash 5 merge` runs a merge. And `gh -R example/repo pr merge` names its repository before the command. A longer short form such as `-Rx` is one word, and `--` ends - the search. + the search. A `limit` of 3 reads the third word a `_GH_WRITE_SUBSUBS` write names. """ - return [args[i] for i in _gh_path_at(args)] + return [args[i] for i in _gh_path_at(args, limit)] -def _gh_path_at(args): +def _gh_path_at(args, limit=2): """The index in `args` of each word `_gh_path` reads.""" at = [] i = 0 - while i < len(args) and len(at) < 2: + while i < len(args) and len(at) < limit: a = args[i] if a == "--": break @@ -282,6 +298,52 @@ def _gh_path_writes(words): return _gh_path_may_be(words, _GH_WRITE_SUBS) +def _gh_path3_writes(args): + """True when a gh argv's first three path words are or may be a write `_GH_WRITE_SUBSUBS` lists.""" + words = _gh_path(args, 3) + subs = _GH_WRITE_SUBSUBS.get(tuple(words[:2]), ()) + return len(words) == 3 and bool(subs) and (words[2] in subs or _is_run_time_word(words[2])) + + +def _gh_names_write(args): + """True when a gh argv's literal words name a write, as `_gh_argv_is_write` reads one. + + That is a command and subcommand the write tables list, or a `gh api` write. A run-time word + where gh's command or subcommand goes does not count, unless a literal write sits behind it + (see `_gh_write_past_run_time`). + """ + if args[:1] == ["api"]: + return _gh_argv_is_write(args) + words = _gh_path(args, 3) + if any(_is_run_time_word(w) for w in words[:2]): + return _gh_write_past_run_time(args) is not None + if len(words) > 1 and words[1] in _GH_WRITE_SUBS.get(words[0], ()): + return True + return len(words) == 3 and words[2] in _GH_WRITE_SUBSUBS.get(tuple(words[:2]), ()) + + +def _gh_write_past_run_time(args, _depth=0): + """The run-time word where gh's command or subcommand goes that a literal write sits behind, or None. + + The shell may expand such a word to a flag with its value, attached or as the next word, as a + `"$R"` holding `--repo=stranger/example` does. gh then reads its command and subcommand past + it, so `gh "$R" pr comment 5` is a comment to a repository the guard cannot read. A `gh api` + argv has its endpoint where a subcommand goes, so it has no such word. + """ + at = [i for i in _gh_path_at(args) if _is_run_time_word(args[i])] + if not at or _depth > 2 or args[:1] == ["api"]: + return None + i = at[0] + for rest in (args[:i] + args[i + 1 :], args[:i] + args[i + 2 :]): + rest = _api_first(rest) + if any(_is_run_time_word(w) for w in _gh_path(rest)): + if _gh_write_past_run_time(rest, _depth + 1) is not None: + return args[i] + elif _gh_names_write(rest): + return args[i] + return None + + def _gh_may_merge(args): """True when a `gh` argv may be `pr merge` through a run-time word the shell rewrites. @@ -2421,6 +2483,46 @@ def _api_first(args): return args +def _gh_invocations(text): + """(the tokens of `text`, each gh invocation in `text` itself as (token index, argv)). + + That is each gh word, its argv read as `_gh_arg_lists` reads it. It is also each run-time word + in a command's place, such as `$G` or `"$(which gh)"`, whose argv names a gh write in literal + words (see `_gh_names_write`), as in `G=gh; $G pr comment 5 -R stranger/example`. A non-gh + command behind such a word whose words read as a gh write is read as one too. + """ + toks = _shell_tokens(text) + out = [] + mask = None + k = 0 + while k < len(toks): + t = toks[k] + if _is_gh_exe(t): + args, end = _collect_arglist(toks, k + 1) + out.append((k, _api_first(args))) + k = end + continue + k += 1 + if "$" not in t and "`" not in t: + continue + if mask is None: + mask = _quoted_mask(text, toks) + if not _is_run_time_command(toks, k - 1, mask): + continue + start = _span_end(toks, k - 1, _region_end(toks, k - 1, mask)) + if start is None: + continue + args = _api_first(_collect_arglist(toks, start)[0]) + if args and _gh_names_write(args): + out.append((k - 1, args)) + return toks, out + + +def _gh_target_arg_lists(text): + """The argv of each gh invocation `_gh_invocations` reads in `text` itself.""" + return [args for _, args in _gh_invocations(text)[1]] + + _SHELL_WRAPPER_EXE = ("sh", "bash", "zsh", "ksh", "dash") @@ -2654,10 +2756,11 @@ def _shell_bodies_at(toks, i): def _all_gh_arg_lists(cmd): """`_gh_arg_lists` for `cmd` itself, plus for every command string a `sh -c`/`bash -c`-style wrapper embeds in it, so a `gh` call hidden behind such a wrapper is scanned exactly like a bare one. + A gh write behind a run-time command word is read too (see `_gh_invocations`). """ - out = list(_gh_arg_lists(cmd)) + out = list(_gh_target_arg_lists(cmd)) for inner in _embedded_wrapper_commands(cmd): - out.extend(_gh_arg_lists(inner)) + out.extend(_gh_target_arg_lists(inner)) return out @@ -2693,16 +2796,21 @@ def _gh_target_readings(argvs): yield args, _GH_MERGE_TEXT_FLAGS -def _gh_write_targets(cmd): +def _gh_write_targets(cmd, environ=None): """Every explicit owner/repo target named in an actual `gh` invocation's own argv (including one embedded in a `sh -c`/`bash -c` wrapper): a `--repo`/`-R` flag value, or a `repos//` API path token. Argv-position parsing, the way `_push_targets` reads a git push target, so a --repo/repos mention that is only prose, inside an unrelated --body/--title value or a commit message, is never - read as one. A `GH_REPO` assignment's value is one too (see `_gh_repo_assignments`). + read as one. A `GH_REPO` value that reaches a gh word is one too (see `_gh_repo_assignments`), and + so is a repository a write's positional argument names (see `_gh_positional_repo_words`). Another + API path naming an owner is one on a `gh api` call (see `_API_OWNER_PATHS`). """ - words = [(w, is_path) for w, is_path, _ in _gh_repo_words(_all_gh_arg_lists(cmd))] - words += [(w, False) for w, _ in _gh_repo_assignments(cmd)] - return [t for w, is_path in words for t in [_named_target(w, is_path)] if t] + argvs = _all_gh_arg_lists(cmd) + words = [(w, is_path, a[:1] == ["api"]) for w, is_path, a in _gh_repo_words(argvs)] + words += [(w, False, False) for w, _, _ in _gh_repo_assignments(cmd, environ)] + targets = [t for w, is_path, api in words for t in _named_targets(w, is_path, api)] + positional = [w for a in argvs for w, _ in _gh_positional_repo_words(a)] + return targets + [t for w in positional for t in [_gh_repo_value_reading(w)] if t] def _gh_repo_words(argvs): @@ -2800,38 +2908,235 @@ def _gh_short_value_letters(args, flags): return "".join(f[1] for f in flags if len(f) == 2) -def _named_target(word, is_path): - """The (owner, repo) a repository word or a normalized API path names, or None.""" +_GH_URL = re.compile( + r"^(?:[a-z][a-z0-9+.\-]*://(?:[^/@]*@)?[^/]+/|[^/@:\s]+@[^/:]+:)(?P[^/]+)/(?P[^/]+)", + re.IGNORECASE, +) +_API_SEGMENT = r"[A-Za-z0-9_.\-]+" +_API_OWNER_PATHS = ( + re.compile(rf"^(?:orgs|users)/(?P{_API_SEGMENT})(?:/|$)"), + re.compile(rf"^user/starred/(?P{_API_SEGMENT})/(?P{_API_SEGMENT})"), + re.compile(rf"^user/(?:following|blocks)/(?P{_API_SEGMENT})(?:/|$)"), + re.compile(rf"^user/memberships/orgs/(?P{_API_SEGMENT})(?:/|$)"), +) +_API_OWNER_SEGMENTS = ( + re.compile(r"^(?:repos|orgs|users)/([^/]*)"), + re.compile(r"^user/(?:starred|following|blocks)/([^/]*)"), + re.compile(r"^user/memberships/orgs/([^/]*)"), + re.compile(r"^([^/]*)"), +) + + +def _gh_repo_value_reading(word): + """The (owner, repo) gh reads a repository word as, a URL or `[HOST/]OWNER/REPO`, else None.""" + m = _GH_URL.match(word) + parts = [m.group("owner"), m.group("repo")] if m else word.split("/") + parts = parts[1:] if len(parts) == 3 else parts + if len(parts) != 2 or not all(parts) or "<" in word: + return None + return parts[0].lower(), parts[1].lower() + + +def _named_targets(word, is_path, api=False): + """Each (owner, repo) a repository word or a normalized API path names. + + A repository word is read as gh reads it (see `_gh_repo_value_reading`), and with its first + part as the owner as well, so the reading gh gives a three-part value only adds a target. An + API path names a `repos/` target, and on a `gh api` call (`api`) any `_API_OWNER_PATHS` one too. + A path naming an owner but no repository names all of the owner's, as `("stranger", "*")`. + """ + if "<" in word: + return [] + out = [] if is_path: m = _REPOS_PATH_TOKEN.match(word) - if m and "<" not in word: - return m.group("owner").lower(), m.group("repo").lower() - return None - if "/" in word and "<" not in word: + if m: + out.append((m.group("owner").lower(), m.group("repo").lower())) + for pattern in _API_OWNER_PATHS if api else (): + m = pattern.match(word) + if m: + out.append((m.group("owner").lower(), (m.groupdict().get("repo") or "*").lower())) + return out + if "/" in word: o, r = word.split("/", 1) - return o.lower(), r.lower() - return None + out.append((o.lower(), r.lower())) + read = _gh_repo_value_reading(word) + return out + ([read] if read and read not in out else []) + + +def _repo_value_owners(word, exact=False): + """Each part of a repository word `_named_targets` reads as an owner, whatever it holds. + + With `exact`, that is only the owner gh reads, or the whole word where gh reads none, since + a run-time word may hold a whole `OWNER/REPO`. + """ + read = _gh_repo_value_reading(word) + m = _GH_URL.match(word) + parts = word.split("/") + owner = m.group("owner") if m else (parts[1] if len(parts) == 3 else parts[0]) + if exact: + return [owner if read or m else word] + return [parts[0], owner] + + +_GH_POSITIONAL_VALUE_FLAGS = { + (cmd, sub): frozenset(flags.split()) + for cmd, sub, flags in ( + ("pr", "comment", "--attach -b --body -F --body-file"), + ("pr", "close", "-c --comment"), + ( + "pr", + "merge", + "-A --author-email -b --body -F --body-file --match-head-commit -t --subject", + ), + ( + "pr", + "edit", + ( + "--add-assignee --add-label --add-project --add-reviewer --attach -B --base " + "-b --body -F --body-file -m --milestone --remove-assignee --remove-label " + "--remove-project --remove-reviewer -t --title" + ), + ), + ("pr", "review", "-b --body -F --body-file"), + ("pr", "reopen", "-c --comment"), + ("pr", "lock", "-r --reason"), + ("pr", "revert", "-b --body -F --body-file -t --title"), + ("issue", "comment", "--attach -b --body -F --body-file"), + ("issue", "close", "-c --comment --duplicate-of -r --reason"), + ( + "issue", + "edit", + ( + "--add-assignee --add-blocked-by --add-blocking --add-label --add-project " + "--add-sub-issue --attach -b --body -F --body-file -m --milestone --parent " + "--remove-assignee --remove-blocked-by --remove-blocking --remove-label " + "--remove-project --remove-sub-issue -t --title --type" + ), + ), + ("issue", "reopen", "-c --comment"), + ("issue", "lock", "-r --reason"), + ("issue", "develop", "-b --base --branch-repo -n --name --worktree"), + ( + "repo", + "create", + ( + "-d --description -g --gitignore -h --homepage -l --license " + "-r --remote -s --source -t --team -p --template" + ), + ), + ( + "repo", + "edit", + ( + "--add-topic --default-branch -d --description -h --homepage " + "--remove-topic --squash-merge-commit-message --visibility" + ), + ), + ("repo", "sync", "-b --branch -s --source"), + ) +} +_GH_REPO_POSITIONAL_SUBS = frozenset( + ("repo", s) for s in ("create", "delete", "edit", "archive", "unarchive", "sync") +) + + +def _gh_positional_repo_words(args): + """Each repository word a gh write's positional arguments name, as (word, argv). + + A `pr` or `issue` write takes its pull request or issue as a URL too, which names the + repository, as in `gh pr comment https://github.com/stranger/example/pull/5`. A number or a + branch there selects inside the repository gh reads otherwise, so it is no target. A wholly + run-time word there may hold a URL too, and is left alone, since it is far more often a number. + A `_GH_REPO_POSITIONAL_SUBS` write takes the repository itself, as `gh repo + edit stranger/example` does. `gh issue transfer` takes its destination second, and + `gh issue develop --branch-repo` names the repository its branch goes to. A flag's value is + passed by `_GH_POSITIONAL_VALUE_FLAGS`, the value-taking flags gh's help lists for each such + write, and any other flag is read as a boolean. + """ + key = tuple(_gh_path(args)) + if len(key) < 2 or key[1] not in _GH_WRITE_SUBS.get(key[0], ()): + return [] + takes = _GH_POSITIONAL_VALUE_FLAGS.get(key, frozenset()) | _REPO_FLAG_BARE + path = set(_gh_path_at(args)) + positional, named, ended = [], [], False + i = 0 + while i < len(args): + a = args[i] + i += 1 + if i - 1 in path: + continue + if ended or not a.startswith("-") or a == "-": + positional.append(a) + continue + if a == "--": + ended = True + continue + if key == ("issue", "develop") and a.partition("=")[0] == "--branch-repo": + named += [a.partition("=")[2]] if "=" in a else args[i : i + 1] + if a in takes: + i += 1 + if key in _GH_REPO_POSITIONAL_SUBS: + named += positional + elif key == ("issue", "transfer"): + named += positional[1:] + positional = positional[:1] + if key[0] in ("pr", "issue"): + named += [w for w in positional if _GH_URL.match(w)] + return [(w, args) for w in named] _REPOS_PATH_OWNER = re.compile(r"^/?repos/[^/]*") -def _gh_run_time_target(cmd): +def _gh_api_run_time_owner(args): + """The endpoint of a `gh api` argv whose owner segment, or first segment, is a run-time word, else None. + + The owner segment is the one an `_API_OWNER_SEGMENTS` path names, as `$O` in `orgs/$O/teams`. A + first segment that is a run-time word, as in `"$URL"`, may hold any owner. + """ + path = _gh_api_path(args) + if path is None: + return None + for pattern in _API_OWNER_SEGMENTS: + m = pattern.match(path) + if m and _is_run_time_word(m.group(1)): + return path + return None + + +def _gh_run_time_target(cmd, environ=None): """The first run-time word a gh write in `cmd` names its repository's owner with, or None. The shell rewrites it before gh reads it, so the target check has nothing to compare. That is - a repo flag's value, the owner segment of a `repos/` API path, or a `GH_REPO` value. A literal - owner is all the check compares, so `--repo "example/$R"` is left alone. So is a read naming - one, as in `gh pr view -R "$T"`, and a heredoc body that is data, wrapper bodies in it included. - """ - argvs = [a for t in _data_free_texts(cmd) for a in _gh_arg_lists(t)] + a repo flag's value, the owner segment of a `repos/` API path, or a `GH_REPO` value. It is also + a repository a positional argument names (see `_gh_positional_repo_words`), the owner segment + or first segment of a `gh api` write's endpoint (see `_gh_api_run_time_owner`), and a word + before a literal write where gh's command goes (see `_gh_write_past_run_time`). A literal owner + is all the check compares, so `--repo "example/$R"` is left alone. So is a read naming one, as + in `gh pr view -R "$T"`, a `GH_REPO` the session's environment holds, and a heredoc body that + is data, wrapper bodies in it included. + """ + argvs = [a for t in _data_free_texts(cmd) for a in _gh_target_arg_lists(t)] for word, is_path, args in _gh_repo_words(argvs): m = _REPOS_PATH_OWNER.match(word) if is_path else None - owner = (m.group(0) if m else "") if is_path else word.partition("/")[0] - if _is_run_time_word(owner) and _gh_argv_is_write(args): + owners = ([m.group(0)] if m else []) if is_path else _repo_value_owners(word) + if any(_is_run_time_word(o) for o in owners) and _gh_argv_is_write(args): return word - for value, args in _gh_repo_assignments(cmd): - if _is_run_time_word(value.partition("/")[0]) and _gh_argv_is_write(args): + for args in argvs: + if not _gh_argv_is_write(args): + continue + hidden = _gh_write_past_run_time(args) or ( + _gh_api_run_time_owner(args) if args[:1] == ["api"] else None + ) + if hidden: + return hidden + for word, _ in _gh_positional_repo_words(args): + if any(_is_run_time_word(o) for o in _repo_value_owners(word, exact=True)): + return word + for value, args, from_env in _gh_repo_assignments(cmd, environ): + owners = [] if from_env else _repo_value_owners(value) + if any(_is_run_time_word(o) for o in owners) and _gh_argv_is_write(args): return value return None @@ -2841,44 +3146,149 @@ def _data_free_texts(cmd): return [t for r in _heredoc_readings(cmd) or [cmd] for t in [r, *_embedded_wrapper_commands(r)]] -def _gh_repo_assignments(cmd): - """Each `GH_REPO` value that reaches a gh word in `cmd`, as (value, argv), wrapper bodies included. +_GH_REPO_PLACEHOLDER = re.compile(r":(?:owner|repo)\b|\{(?:owner|repo)\}") + + +def _gh_repo_assignments(cmd, environ=None): + """Each `GH_REPO` value that reaches a gh word in `cmd`, as (value, argv, from_env), wrapper bodies included. gh reads its repository from GH_REPO where no repo flag names one, so a gh word whose argv - names one takes none. `gh api` reads it only to fill an `{owner}` or `{repo}` placeholder. A + names one takes none. `gh api` reads it only to fill a `_GH_REPO_PLACEHOLDER` placeholder, the + older `:owner` spelling included. A value reaches a gh word from the prefix of the simple command it runs (see `_command_prefix`), - or else from the latest `export` before it. A wrapper body takes what - `_wrapper_env_assignments` says reaches a wrapper, the latest such value for every body. A - heredoc body that is data is left out, and an argument such as `echo GH_REPO=x` does not count. - """ + or else from the shell's state before it (see `_gh_repo_states`), which starts from a GH_REPO + the session's environment holds (`from_env`). A wrapper body takes what + `_wrapper_env_assignments` says reaches a wrapper, the latest such value for every body, and + also the value reaching its own wrapper alone (see `_gh_wrapper_repo_values`). A heredoc body + that is data is left out, and an argument such as `echo GH_REPO=x` does not count. + """ + env = (environ if environ is not None else os.environ).get("GH_REPO", "") + base = [(env, True)] if env else [] out = [] for reading in _heredoc_readings(cmd) or [cmd]: + out += _gh_word_repo_values(reading, base) toks = _shell_tokens(reading) - out += _gh_word_repo_values(toks, []) reaching = _wrapper_env_assignments(toks, lambda body: bool(_all_gh_arg_lists(body))) - inherited = [v for name, v in reaching if name == "GH_REPO"][-1:] + inherited = [(v, False) for name, v in reaching if name == "GH_REPO"][-1:] for inner in _embedded_wrapper_commands(reading): - out += _gh_word_repo_values(_shell_tokens(inner), inherited) + out += _gh_word_repo_values(inner, inherited or base) + out += _gh_wrapper_repo_values(reading, base) return out -def _gh_word_repo_values(toks, inherited): - """The `GH_REPO` value reaching each gh word in `toks`, as (value, argv), past `inherited`.""" - out, exported = [], list(inherited) +def _export_words(toks, x): + """(the `NAME=value` arguments as (name, value), the bare names) of the `export` at `toks[x]`.""" + pairs, names = [], [] + for word in toks[x + 1 :]: + if _is_shell_op(word): + break + m = _ENV_ASSIGN_RE.match(word) + if m: + pairs.append(m.groups()) + elif re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", word): + names.append(word) + elif not word.startswith("-"): + break + return pairs, names + + +def _plain_assignments(toks, x): + """The `NAME=value` words starting at `toks[x]` that stand as a command of their own, as (name, value).""" + if x and not (_is_separator(toks[x - 1]) or toks[x - 1] in _COND_PRECEDERS): + return [] + out = [] + for word in toks[x:]: + m = _ENV_ASSIGN_RE.match(word) + if not m: + return out if _is_separator(word) else [] + out.append(m.groups()) + return out + + +def _gh_repo_states(toks, inherited): + """The `GH_REPO` values a command at each index of `toks` runs under, past `inherited`, as a list each. + + Each value is (value, from_env). The latest `export GH_REPO=v` sets the value. So does a plain + `GH_REPO=v` standing as its own command once GH_REPO is exported, by an `export GH_REPO` + before or after it or by `inherited`, whose environment exports it. That plain value is read + beside the latest exported one, so the reading can only add a value. A value from the session's + environment is kept beside both, since an export may sit where it never runs, as in a subshell + or an untaken branch. Only a prefix replaces it. + """ + exported, marked, plain, states = list(inherited), bool(inherited), None, [] + env = [v for v in inherited if v[1]] for x, t in enumerate(toks): + values = exported[-1:] + [v for v in env if v not in exported[-1:]] + states.append(values + ([plain] if marked and plain else [])) if t == "export" and _opens_command(toks, x): - exported += [v for name, v in _export_assignments(toks, x) if name == "GH_REPO"] + pairs, names = _export_words(toks, x) + values = [(v, False) for name, v in pairs if name == "GH_REPO"] + exported += values + plain = None if values else plain + marked = marked or bool(values) or "GH_REPO" in names continue - if not _is_gh_exe(t): + values = [v for name, v in _plain_assignments(toks, x) if name == "GH_REPO"] + plain = (values[-1], False) if values else plain + return states + + +def _gh_word_repo_values(text, inherited): + """The `GH_REPO` value reaching each gh word in `text` itself, as (value, argv, from_env), past `inherited`. + + A gh write behind a run-time command word is one such word (see `_gh_invocations`). A gh word + runs as its own command, or behind a launcher such as `flock` or `find -exec` (see + `_launcher_prefix`). Its prefix reaches it there, or else the shell's state does. + """ + toks, invocations = _gh_invocations(text) + states = _gh_repo_states(toks, inherited) + out = [] + for x, argv in invocations: + if any(not is_path for _w, is_path, _a in _gh_repo_words([argv])): + continue + if argv[:1] == ["api"] and not any(_GH_REPO_PLACEHOLDER.search(a) for a in argv): continue reached, pairs = _command_prefix(toks, x) - argv = _api_first(_collect_arglist(toks, x + 1)[0]) - if not reached or any(not is_path for _w, is_path, _a in _gh_repo_words([argv])): + if not reached: + reached, pairs = _launcher_prefix(toks, x) + if not reached: continue - if argv[:1] == ["api"] and not any("{owner}" in a or "{repo}" in a for a in argv): + prefix = [(v, False) for name, v in pairs if name == "GH_REPO"] + out += [(v, argv, env) for v, env in prefix or states[x]] + return out + + +def _gh_wrapper_repo_values(text, inherited, _depth=0): + """The `GH_REPO` value reaching each gh word in a wrapper body in `text`, read for that wrapper alone. + + A wrapper's bodies run under its own GH_REPO prefix, or else the value its command runs under + (see `_gh_repo_states`), and a wrapper nested in one inherits from it in turn. A substitution + body the tokenizer keeps inside one word, as in `"$(...)"`, runs under the value at each word + opening it, or at every word where none is found. + """ + if _depth > _WRAPPER_NESTING_LIMIT: + return [] + toks = _shell_tokens(text) + states = _gh_repo_states(toks, inherited) + out = [] + for x in range(len(toks)): + found = _shell_bodies_at(toks, x) + if not found: continue - prefix = [v for name, v in pairs if name == "GH_REPO"] - out += [(v, argv) for v in prefix or exported[-1:]] + reached, pairs = _command_prefix(toks, x) + if not reached: + reached, pairs = _launcher_prefix(toks, x) + prefix = [(v, False) for name, v in pairs if name == "GH_REPO"] if reached else [] + reach = prefix or states[x] + for body in found[0]: + out += _gh_word_repo_values(body, reach) + out += _gh_wrapper_repo_values(body, reach, _depth + 1) + for body in _substitution_bodies(text): + at = [k for k, t in enumerate(toks) if "$(" + body in t or "`" + body in t] or range( + len(toks) + ) + reach = list(dict.fromkeys(v for k in at for v in states[k])) if toks else list(inherited) + out += _gh_word_repo_values(body, reach) + out += _gh_wrapper_repo_values(body, reach, _depth + 1) return out @@ -4919,7 +5329,8 @@ def _check_bypass_flags(cmd): "to `pr merge` itself, or name a repository the target check cannot read. Failing " "closed. Write every word of the merge out. Only a quoted value of an option other " 'than --repo, as in --body "$BODY", may be a run-time word, and a quoted one where ' - "gh's command or subcommand goes when no --admin is among the words." + _handoff(cmd) + "gh itself, gh's command, or its subcommand goes when no --admin is among the words." + + _handoff(cmd) ) # The --no-verify flag and `commit -n` skip the git hooks, so they only matter as an actual argument to a git commit or push. # Other tools use --no-verify for unrelated things, and shlex keeps a quoted mention out of the argv. @@ -6663,7 +7074,8 @@ def classify( ) # 2. Literal node id in a mutation - if _GRAPHQL.search(cmd) and _MUTATION.search(cmd): + graphql = any(_gh_api_path(a) == "graphql" for a in _all_gh_arg_lists(cmd)) + if (_GRAPHQL.search(cmd) or graphql) and _MUTATION.search(cmd): for m in _FIELD_ASSIGN.finditer(cmd): val = m.group("v").strip("'\"") if val.startswith(("$", "${")): @@ -6682,7 +7094,7 @@ def classify( origin = _origin_owner_repo(cwd) # `_gh_write_targets` reads argv position within each real `gh` invocation, so a compound command carrying one target per invocation still has every one read (the write after `&&` is not skipped). # A --repo/repos// mention living inside an unrelated --body/--title/-f value, or in a non-gh command entirely, is not read as a target. - targets = _gh_write_targets(cmd) + targets = _gh_write_targets(cmd, environ) # This only runs when origin resolves, meaning a git checkout, since with no project context there is nothing to compare an explicit target against, so the check is skipped and rules 1 and 2 still apply. # A node-id target is invisible here regardless, which is what rule 2 guards. if origin: @@ -6700,7 +7112,7 @@ def classify( "Safety', or the same section of the user-level CLAUDE.md where the repo has no " "GOVERNANCE.md." ) - hidden_target = _gh_run_time_target(cmd) + hidden_target = _gh_run_time_target(cmd, environ) if hidden_target: return "deny", ( f"This write names its repository with a run-time word, `{hidden_target}`, which " @@ -7035,6 +7447,306 @@ def classify( ] +_STRANGER_GH_REPO = {"GH_REPO": "stranger/example"} +_GH_TARGET_CASES: list[tuple[str, dict[str, str], str, str]] = [ + ( + "gh pr comment 5 --body hi", + _STRANGER_GH_REPO, + "deny", + "a GH_REPO the session's environment holds", + ), + ( + "bash -c 'gh issue comment 5 --body hi'", + _STRANGER_GH_REPO, + "deny", + "an environment GH_REPO in a wrapper", + ), + ( + "gh api -X POST repos/{owner}/{repo}/issues/1/comments -f body=x", + _STRANGER_GH_REPO, + "deny", + "an environment GH_REPO filling api placeholders", + ), + ( + 'GH_REPO="$T"; gh pr comment 5 --body hi', + _STRANGER_GH_REPO, + "deny", + "a run-time plain GH_REPO, exported", + ), + ("gh pr view 5", _STRANGER_GH_REPO, "allow", "an environment GH_REPO on a read"), + ( + "flock /tmp/l gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "an environment GH_REPO behind a launcher", + ), + ( + "find . -name x -exec gh pr comment 5 -b x +", + _STRANGER_GH_REPO, + "deny", + "an environment GH_REPO behind find -exec", + ), + ( + "if false; then export GH_REPO=example/repo; fi; gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "an export in an untaken branch does not replace an environment GH_REPO", + ), + ( + "GH_REPO=example/repo bash -c 'gh pr comment 5 -b x'", + _STRANGER_GH_REPO, + "allow", + "a wrapper prefix replaces an environment GH_REPO", + ), + ("echo gh pr comment 5 -b x", _STRANGER_GH_REPO, "allow", "a gh word that is an argument"), + ( + "false && export GH_REPO=example/repo; gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "a conditional export does not replace an environment GH_REPO", + ), + ( + "export GH_REPO=example/repo & gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "a backgrounded export does not replace an environment GH_REPO", + ), + ( + "export GH_REPO=example/repo | cat; gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "an export in a pipeline does not replace an environment GH_REPO", + ), + ( + "case a in a) true;; esac; (true; export GH_REPO=example/repo); gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "a subshell export past a case does not replace an environment GH_REPO", + ), + ( + "f() { export GH_REPO=example/repo; }; gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "a function's export does not replace an environment GH_REPO", + ), + ( + "(export GH_REPO=example/repo; gh pr view 1); gh pr comment 5 -b x", + _STRANGER_GH_REPO, + "deny", + "a subshell export does not replace an environment GH_REPO", + ), + ( + 'echo "run: gh pr comment 5 -b hi"; export GH_REPO=stranger/example; o="$(gh pr comment 5 -b hi)"', + {}, + "deny", + "a substitution runs under the value at the word opening it", + ), + ( + "gh repo delete ssh://git@github.com/stranger/example.git --yes", + {}, + "deny", + "an ssh URL repo positional", + ), + ( + "gh issue transfer 1 git@github.com:stranger/example.git", + {}, + "deny", + "an scp-form destination", + ), + ( + "$G api graphql -f query='mutation($i: ID!) { m(input: {id: $i}) { x } }' -F i=PR_kwDOABCDEFGHIJKLMNOP", + {}, + "deny", + "a literal node id in a mutation behind a run-time command word", + ), + ( + "gh pr comment 5 -R example/repo --body hi", + _STRANGER_GH_REPO, + "allow", + "a repo flag overrides it", + ), + ( + "GH_REPO=example/repo gh pr comment 5 --body hi", + _STRANGER_GH_REPO, + "allow", + "a prefix overrides it", + ), + ("gh pr comment 5 --body hi", {"GH_REPO": "example/other"}, "allow", "a sibling one"), + ('gh "$R" pr comment 5 --body hi', {}, "deny", "a quoted run-time word before pr comment"), + ('gh pr "$R" comment 5 --body hi', {}, "deny", "a quoted run-time word before comment"), + ('gh "$F" x pr comment 5 --body hi', {}, "deny", "a run-time flag taking the next word"), + ( + 'gh "$R" api -X POST repos/example/repo/issues -f title=t', + {}, + "deny", + "a run-time word before an api write", + ), + ('gh "$R" pr view 5', {}, "allow", "a run-time word before a read"), + ('gh pr "$S" 5', {}, "allow", "a quoted run-time subcommand with no write behind it"), + ( + "GH_REPO=stranger/example; export GH_REPO; gh pr comment 5 --body hi", + {}, + "deny", + "a bare export of a plain GH_REPO", + ), + ( + "export GH_REPO; GH_REPO=stranger/example; gh pr comment 5 --body hi", + {}, + "deny", + "a plain GH_REPO after a bare export", + ), + ( + "export A GH_REPO=stranger/example; gh pr comment 5 --body hi", + {}, + "deny", + "a GH_REPO export past a bare name", + ), + ( + "GH_REPO=stranger/example; gh pr comment 5 --body hi", + {}, + "allow", + "a plain GH_REPO never exported", + ), + ( + "G=gh; $G pr comment 5 -R stranger/example --body hi", + {}, + "deny", + "a write behind a run-time command word", + ), + ( + '"$(command -v gh)" issue comment 5 --repo stranger/example --body hi', + {}, + "deny", + "a write behind a quoted command substitution", + ), + ('"$G" pr comment 5 -R "$T" --body hi', {}, "deny", "a run-time repo there"), + ("GH_REPO=stranger/example $G pr comment 5 --body hi", {}, "deny", "a GH_REPO prefix there"), + ("$G pr comment 5 --body hi 2>/dev/null", {}, "deny", "a suppressed write there"), + ('"$G" pr view 5 -R stranger/example', {}, "allow", "a read there"), + ("$G pr comment 5 -R example/repo --body hi", {}, "allow", "an origin write there"), + ('"$EDITOR" notes.md', {}, "allow", "a run-time command word naming no gh write"), + ( + ( + "GH_REPO=stranger/example bash -c 'gh issue comment 5 --body hi'; " + "GH_REPO=example/repo bash -c 'gh pr view 5'" + ), + {}, + "deny", + "a GH_REPO is read for the wrapper it prefixes", + ), + ( + 'export GH_REPO=stranger/example; echo "$(gh pr comment 5 --body hi)"', + {}, + "deny", + "an exported GH_REPO reaches a quoted substitution", + ), + ( + 'GH_REPO=stranger/example echo "$(gh pr comment 5 --body hi)"', + {}, + "allow", + "a prefix on the command holding a substitution does not reach it", + ), + ( + "GH_REPO=stranger/example gh api -X POST repos/:owner/:repo/issues/1/comments -f body=x", + {}, + "deny", + "a GH_REPO filling the older :owner placeholders", + ), + ( + "gh pr comment 5 --repo example/stranger/x -b x", + {}, + "deny", + "a host-prefixed repo value is read by its middle owner", + ), + ( + "GH_REPO=example/stranger/x gh pr comment 5 -b x", + {}, + "deny", + "a host-prefixed GH_REPO is read by its middle owner", + ), + ( + "gh pr comment https://github.com/stranger/example/pull/5 --body x", + {}, + "deny", + "a pull request URL naming another owner", + ), + ( + "gh issue edit 5 https://github.com/stranger/example/issues/6 --add-label x", + {}, + "deny", + "an issue URL among several", + ), + ( + 'gh pr comment "https://github.com/$O/example/pull/5" --body x', + {}, + "deny", + "a URL with a run-time owner", + ), + ("gh repo edit stranger/example --description x", {}, "deny", "a repo write's positional"), + ("gh repo create stranger/example --private", {}, "deny", "a repo create under another owner"), + ("gh repo archive https://github.com/stranger/example -y", {}, "deny", "a repo URL positional"), + ('gh repo edit "$R" --description x', {}, "deny", "a run-time repo positional"), + ("gh issue transfer 5 stranger/example", {}, "deny", "a transfer's destination"), + ('gh issue transfer 5 "$DEST"', {}, "deny", "a run-time transfer destination"), + ("gh issue develop 5 --branch-repo=stranger/example", {}, "deny", "a develop branch repo"), + ("gh workflow run ci.yml -R stranger/example", {}, "deny", "a workflow run"), + ("gh run rerun 123 -R stranger/example", {}, "deny", "a run rerun"), + ("gh cache delete --all -R stranger/example", {}, "deny", "a cache delete"), + ("gh pr update-branch 5 -R stranger/example", {}, "deny", "a pr update-branch"), + ("gh label clone example/repo -R stranger/example", {}, "deny", "a label clone"), + ("gh repo deploy-key add k.pub -R stranger/example", {}, "deny", "a deploy key add"), + ( + "gh pr comment https://github.com/example/repo/pull/5 --body x", + {}, + "allow", + "a pull request URL naming the origin", + ), + ('gh pr comment "$PR" --body x', {}, "allow", "a run-time pull request selector"), + ( + "gh pr comment 5 --body https://github.com/stranger/example/pull/5", + {}, + "allow", + "a URL in a body", + ), + ("gh repo create newrepo --private", {}, "allow", "a repo create naming no owner"), + ( + 'gh repo create "example/$NAME" --private', + {}, + "allow", + "a run-time name under the origin owner", + ), + ( + "gh repo edit example/other --description stranger/example", + {}, + "allow", + "an owner/repo-shaped description", + ), + ("gh repo fork stranger/example", {}, "allow", "a fork reads its source"), + ("gh label clone stranger/example", {}, "allow", "a label clone reads its source"), + ("gh repo deploy-key list -R stranger/example", {}, "allow", "a deploy key list"), + ("gh api -X POST orgs/stranger/teams -f name=x", {}, "deny", "an orgs/ api write"), + ("gh api -X PUT user/starred/stranger/example", {}, "deny", "a star"), + ("gh api -X PUT user/following/stranger", {}, "deny", "a follow"), + ( + "gh api -X POST https://api.github.com/orgs/stranger/repos -f name=x", + {}, + "deny", + "an orgs/ URL", + ), + ('gh api -X POST "$URL" -f body=x', {}, "deny", "an api write whose endpoint is run-time"), + ('gh api -X POST "orgs/$ORG/teams" -f name=x', {}, "deny", "a run-time orgs/ owner"), + ("gh api orgs/stranger/teams", {}, "allow", "an orgs/ read"), + ('gh api "$URL"', {}, "allow", "a run-time endpoint on a read"), + ( + "gh api -X POST orgs/example/teams -f name=x", + {}, + "allow", + "an orgs/ write under the origin owner", + ), + ('gh api -X POST "orgs/example/$T" -f x=y', {}, "allow", "a run-time segment past the owner"), +] + + # Rule-5 cases, covering the hand-rolled reply/resolve denial and its cross-owner grant escape. # Each carries the environment the grant is read from, matching the _SCOPE_CASES convention below. _REPLY_RESOLVE_CASES = [ @@ -13758,6 +14470,19 @@ def _selftest(): if got != want: ok = False print(f" {mark} [{got:5}] want={want:5} {label}") + for cmd, env, want, label in _GH_TARGET_CASES: + got, _ = classify( + cmd, + origin=("example", "repo"), + current_branch="feature/x", + rules_lookup=lambda br: set(), + environ=env, + primary_checkout_lookup=lambda d: False, + ) + mark = "ok " if got == want else "FAIL" + if got != want: + ok = False + print(f" {mark} [{got:5}] want={want:5} {label}") for cmd, env, want, label in _REPLY_RESOLVE_CASES: got, _ = classify( cmd,