feat(groom): optional environment input on the three token-minting jobs

.github/groom/tests/test_environment_binding.py

@@ -0,0 +1,156 @@
+"""The optional `environment` input, and the boundary it must never cross.
+
+`groom.yml` lets a caller bind a GitHub environment so the bot App key can be an
+ENVIRONMENT secret behind a deployment-branch policy instead of a repository
+secret every branch can read. Two properties make that safe, and both are
+invisible in review once the file is 3000 lines long:
+
+  * the default is `''` — an empty environment name binds nothing, so the eight
+    existing callers, none of which pass the input, keep today's behavior; and
+  * only the jobs that MINT the bot token bind it. The finder / verifier /
+    builder jobs run a model over untrusted repository content, and putting one
+    of those inside a credentialed environment is exactly the boundary the
+    split-job topology exists to hold.
+
+Both regress silently: a hardcoded environment name breaks every caller at
+startup with no local signal, and an `environment:` added to an agent job is one
+green line in a diff. Asserted as text rather than parsed — PyYAML is not stdlib
+and this repo is stdlib-only (same reasoning as test_interval.py's literal pins).
+
+The two matchers below are deliberately SHAPE-based rather than exact strings.
+An earlier draft tested for the literal `${{ secrets.BOT_APP_PRIVATE_KEY }}` and
+for `^    environment:` at exactly four spaces, which meant a benign rewrite —
+`${{secrets.BOT_APP_PRIVATE_KEY}}` with the spaces closed up, the key aliased
+through a job-level `env:`, or a job re-indented — silently dropped the job out
+of BOTH the minting set and the bound set at once, leaving every assertEqual
+green while the property they exist to protect was gone.
+"""
+
+import os
+import re
+import unittest
+
+BINDING = "environment: ${{ inputs.bot_app_id != '' && inputs.environment || '' }}"
+# Any reference to the bot App key, however the expression is spaced or wrapped,
+# and wherever in the job it appears (a step's `with:`, or a job-level `env:`).
+SECRET_RE = re.compile(r"secrets\s*\.\s*BOT_APP_PRIVATE_KEY")
+# A job-level `environment:` key at ANY indentation — YAML does not require the
+# two-space-per-level style this file happens to use.
+ENV_KEY_RE = re.compile(r"(?m)^\s+environment\s*:")
+# The jobs that run an agent over untrusted repo content. Named explicitly, not
+# derived, so deleting the binding from a credentialed job cannot silently
+# shrink this set too.
+AGENT_JOBS = ("audit_find", "audit_verify", "build")
+MINTING_JOBS = ["build_pr", "build_select", "file"]
+
+
+def _workflow_text():
+    wf = os.path.join(os.path.dirname(__file__), "..", "..", "workflows", "groom.yml")
+    with open(wf, encoding="utf-8") as f:
+        return f.read()
+
+
+def _jobs_section(text):
+    """Everything from `jobs:` to EOF — excludes the `on:` input declaration,
+    which is also spelled `environment:` and would otherwise match ENV_KEY_RE."""
+    return text.split("\njobs:\n", 1)[1]
+
+
+def _job_blocks(text):
+    """Map job name -> that job's block, from `jobs:` to EOF."""
+    body = _jobs_section(text)
+    blocks = {}
+    for block in re.split(r"(?m)^  (?=[A-Za-z_][A-Za-z0-9_-]*:\s*$)", body):
+        name = block.split(":", 1)[0].strip()
+        if name and re.fullmatch(r"[A-Za-z_][A-Za-z0-9_-]*", name):
+            blocks[name] = block
+    return blocks
+
+
+class EnvironmentInputTest(unittest.TestCase):
+    def setUp(self):
+        self.text = _workflow_text()
+        self.jobs = _job_blocks(self.text)
+
+    def test_the_input_is_optional_and_defaults_to_empty(self):
+        # The whole no-op-for-existing-callers claim rests on this default.
+        decl = re.search(
+            r"(?ms)^      environment:\n(.*?)(?=^      [A-Za-z_])", self.text
+        )
+        self.assertIsNotNone(decl, "no `environment:` input declared in groom.yml")
+        body = decl.group(1)
+        self.assertIn("type: string", body)
+        self.assertIn("required: false", body)
+        self.assertRegex(body, r"(?m)^        default: ''$")
+
+    def test_every_job_that_mints_the_bot_token_binds_the_environment(self):
+        minting = sorted(n for n, b in self.jobs.items() if SECRET_RE.search(b))
+        self.assertEqual(
+            minting, MINTING_JOBS,
+            "the set of jobs reading the bot App key changed — re-check the binding",
+        )
+        for name in minting:
+            with self.subTest(job=name):
+                self.assertIn(BINDING, self.jobs[name])
+
+    def test_no_agent_job_sits_inside_a_credentialed_environment(self):
+        # The security boundary: a job that reads untrusted repo content with a
+        # model must never be able to reach an environment's secrets.
+        for name in AGENT_JOBS:
+            with self.subTest(job=name):
+                self.assertIn(name, self.jobs, "agent job missing from groom.yml")
+                self.assertIsNone(
+                    ENV_KEY_RE.search(self.jobs[name]),
+                    "an agent job must not bind a GitHub environment",
+                )
+
+    def test_only_the_minting_jobs_bind_anything_at_all(self):
+        # The converse of the two above, so a binding added to a NEW uncredentialed
+        # job (a future gate, a summary job) is caught rather than assumed benign.
+        bound = sorted(n for n, b in self.jobs.items() if ENV_KEY_RE.search(b))
+        self.assertEqual(bound, MINTING_JOBS)
+
+    def test_the_binding_is_the_input_and_never_a_hardcoded_name(self):
+        # A literal name here would bind an environment that does not exist in a
+        # caller's repo, failing every existing caller at startup.
+        lines = re.findall(r"(?m)^\s+environment\s*:.*$", _jobs_section(self.text))
+        self.assertEqual(len(lines), len(MINTING_JOBS))
+        for line in lines:
+            with self.subTest(line=line):
+                self.assertEqual(line.strip(), BINDING)
+
+    def test_the_binding_is_dropped_when_no_bot_app_is_configured(self):
+        # With no App configured there is no credential for an environment to
+        # guard, and binding one anyway would park the job behind protection
+        # rules for nothing — a denying rule would then drop the run's findings
+        # after the audit had already been billed. The BINDING constant above
+        # already pins the guarded expression on all three jobs; this test pins
+        # the invariant that makes dropping it correct.
+        #
+        # `build_select` and `file` mint OPTIONALLY, so their mint step carries
+        # the same `bot_app_id != ''` condition the binding does.
+        for name in ("build_select", "file"):
+            with self.subTest(job=name):
+                mint = re.search(
+                    r"(?ms)^      - name: Mint bot-identity token.*?(?=^      - name: )",
+                    self.jobs[name],
+                )
+                self.assertIsNotNone(mint, "no mint step found")
+                self.assertRegex(
+                    mint.group(0), r"if: \$\{\{ inputs\.bot_app_id != '' \}\}"
+                )
+        # `build_pr` mints UNCONDITIONALLY, which is safe only because it is
+        # reachable in builder mode alone and `build_select` hard-fails a
+        # `builder: true` run that set no bot_app_id. If that validation ever
+        # goes away, build_pr's unguarded mint becomes reachable with an empty
+        # key and the dropped binding stops being a no-op.
+        self.assertIn(
+            'if [ "$BUILDER" = "true" ] && [ -z "$BOT_APP_ID" ]; then',
+            self.jobs["build_select"],
+            "build_select no longer rejects builder:true without bot_app_id — "
+            "build_pr's unconditional mint depends on that check",
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()

.github/workflows/groom.yml

@@ -114,6 +114,18 @@ name: Groom (reusable)
 #       with:
 #         # Post/act issues as cloud-code-bot instead of github-actions[bot].
 #         bot_app_id: ${{ vars.APP_ID }}
+#         # OPTIONAL: bind a GitHub environment on the three credentialed jobs
+#         # (build_select / file / build_pr) so the bot App key can live as an
+#         # ENVIRONMENT secret with a deployment-branch policy instead of a
+#         # repository secret. The environment must hold a secret named exactly
+#         # BOT_APP_PRIVATE_KEY. It is an ordinary `with:` string input — pass
+#         # the name (or a `vars.` expression), never a secret. Create it (and
+#         # its branch policy) BEFORE setting this — a missing environment is
+#         # auto-created with no rules, so a typo fails open. Use a DEDICATED
+#         # environment holding only this key, and branch policies only: a
+#         # required-reviewer or wait-timer rule stalls groom permanently.
+#         # See docs/callers/groom.md, "Scoping the bot key to an environment".
+#         # environment: bot-main
 #         # No workflows_ref: the briefs + ledger auto-load from the same commit
 #         # this `uses:` pin resolves to (job.workflow_sha — the runner v2.334.0+
 #         # accessor, not the empty `github.job_workflow_sha`). Set it only to
@@ -393,6 +405,41 @@ on:
         type: string
         required: false
         default: ''
+      environment:
+        description: >-
+          GitHub environment (in the CALLER repo) that the three credentialed
+          jobs (build_select, file, build_pr) bind. Lets a caller keep
+          BOT_APP_PRIVATE_KEY as an ENVIRONMENT secret with a main-only
+          deployment-branch policy instead of a repository secret readable by
+          every branch. Empty (the default) binds no environment — existing
+          callers are unaffected, and the binding is also dropped when
+          bot_app_id is unset, since there is then no token to guard. Because
+          the caller's `secrets:` mapping is evaluated in the CALLER job (which
+          cannot itself carry `environment:`), the environment must hold a
+          secret named exactly BOT_APP_PRIVATE_KEY — GitHub then uses the
+          environment's secret instead of the passed value ("If you include
+          `environment` in the reusable workflow at the job level, the
+          environment secret will be used, and not the secret passed from the
+          caller workflow" — docs.github.com, Reuse workflows). Never applied
+          to the agent jobs — they read untrusted repo content and must not sit
+          inside a credentialed environment. THREE constraints on what you
+          point this at, all covered in docs/callers/groom.md: (1) use a
+          DEDICATED environment holding only this key — binding is
+          all-or-nothing, so every secret and variable in the environment is
+          injected into these jobs, build_pr included, which applies a
+          model-authored patch; (2) DEPLOYMENT-BRANCH POLICIES ONLY — a
+          required-reviewer or wait-timer rule PAUSES build_select, and this
+          workflow's concurrency group (cancel-in-progress: false) then parks
+          every later tick behind it, stopping groom for that repo with no
+          failure and no log; build_pr is additionally a max-parallel-1 matrix,
+          so it raises ONE deployment PER finding, not one per run; and (3)
+          CREATE THE ENVIRONMENT AND ITS POLICY FIRST — GitHub creates a
+          referenced-but-missing environment on demand with no rules and no
+          secrets, so a typo'd name fails OPEN: a fully green run, still on the
+          repository-level key, with the gate you think you have absent.
+        type: string
+        required: false
+        default: ''
       builder:
         description: >-
           Opt-in AUTO-BUILDER ("split C", BE-4003). When true, the top
@@ -1975,6 +2022,16 @@ jobs:
     needs: [gate, audit_verify]
     if: needs.gate.outputs.should_run == 'true' && needs.audit_verify.outputs.have_findings == 'true'
     runs-on: ubuntu-latest
+    # Optional caller-side gate on the bot App key: '' (the default) binds no
+    # environment, so existing callers are unaffected. Bound ONLY by the jobs
+    # that mint the bot token — never by the agent jobs, which run a model over
+    # untrusted repo content and must stay outside a credentialed environment.
+    # Also '' when bot_app_id is unset: the mint step below is itself gated on
+    # `bot_app_id != ''`, so with no App configured there is no credential for
+    # an environment to guard, and binding one anyway would put this job behind
+    # protection rules for nothing — a denying rule would then drop the run's
+    # findings after the audit had already been billed.
+    environment: ${{ inputs.bot_app_id != '' && inputs.environment || '' }}
     timeout-minutes: 15
     permissions:
       contents: read
@@ -2195,6 +2252,16 @@ jobs:
     needs: [gate, build_select]
     if: needs.gate.outputs.should_run == 'true' && needs.build_select.outputs.have_file == 'true'
     runs-on: ubuntu-latest
+    # Optional caller-side gate on the bot App key: '' (the default) binds no
+    # environment, so existing callers are unaffected. Bound ONLY by the jobs
+    # that mint the bot token — never by the agent jobs, which run a model over
+    # untrusted repo content and must stay outside a credentialed environment.
+    # Also '' when bot_app_id is unset: the mint step below is itself gated on
+    # `bot_app_id != ''`, so with no App configured there is no credential for
+    # an environment to guard, and binding one anyway would put this job behind
+    # protection rules for nothing — a denying rule would then drop the run's
+    # findings after the audit had already been billed.
+    environment: ${{ inputs.bot_app_id != '' && inputs.environment || '' }}
     timeout-minutes: 15
     permissions:
       contents: read
@@ -2936,6 +3003,15 @@ jobs:
     # never uploaded its result simply fails its own build_pr cell (fail-fast off).
     if: ${{ !cancelled() && needs.gate.outputs.should_run == 'true' && needs.build_select.outputs.have_build == 'true' }}
     runs-on: ubuntu-latest
+    # Optional caller-side gate on the bot App key: '' (the default) binds no
+    # environment, so existing callers are unaffected. Bound ONLY by the jobs
+    # that mint the bot token — never by the agent jobs, which run a model over
+    # untrusted repo content and must stay outside a credentialed environment.
+    # Also '' when bot_app_id is unset — kept identical to the other two
+    # bindings, though it cannot fire here: build_pr is reachable only in
+    # builder mode, and build_select hard-fails a `builder: true` run that set
+    # no bot_app_id, so the mint step below can afford to be unconditional.
+    environment: ${{ inputs.bot_app_id != '' && inputs.environment || '' }}
     # 15 -> 20. Headroom only, and deliberately the LAST line of defence rather
     # than the fix: the 2026-08-04 cancellation was a hung fetch that produced
     # zero bytes, so a bigger budget alone would just have bought a longer hang.