From 48201d4773a4e883429560849b53b1ad0948b37b Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 03:48:51 -0500 Subject: [PATCH 01/14] feat(policy): make deployment policy capability-aware and isolate privileged roles Declare organization.github_plan (free/team/enterprise, omitted = free, fail closed) and gate each environment's approval on a mechanism the plan can actually provide: github-environment requires team/enterprise because GitHub Free private repositories have no protected Environments; manual-external requires recorded exact-head approval_evidence that never names ordinary-CI state as its own approver. Host-group roles become logical inventory labels (deployment, persistent-testing, image-build) pending core ci-fleet #22/#23; the validator rejects routing-label reuse across pools and collisions with privileged host-group identities, and environments may target only deployment-role host groups. Schema, initializer (--github-plan), both example fleets (free/manual and team/environment paths), regression tests (16 new), and documentation are updated together per AGENTS.md. Closes #11 --- README.md | 16 ++++- examples/multi-host/fleet.json | 10 ++- fleet.json | 10 ++- fleet.schema.json | 21 ++++++- scripts/init.py | 7 +++ scripts/test_policy.py | 112 +++++++++++++++++++++++++++++++++ scripts/validate.py | 68 ++++++++++++++++++-- 7 files changed, 232 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 2df86ad..a8885e5 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,16 @@ flowchart LR ./scripts/validate.sh --strict ``` -5. Configure secret **values** in GitHub Environments, root-owned host files, or an external secret manager. The repository stores only names such as `DEPLOY_AUTH`. +5. Configure secret **values** in GitHub Environments (only if your plan provides protected Environments for private repositories — GitHub Free does not), root-owned host files, or an external secret manager. The repository stores only names such as `DEPLOY_AUTH`. + +## Capability-aware deployment policy + +`organization.github_plan` declares what your GitHub plan can actually provide: `free`, `team`, or `enterprise`. Omitted means `free` (fail closed). Each environment then names its real approval gate: + +- `approval_mechanism: github-environment` requires a plan that supports protected Environments for private repositories. The validator rejects this on `free` or when the plan is undeclared. +- `approval_mechanism: manual-external` is the fallback for everyone else. When `requires_approval` is true, `approval_evidence` must record where the exact reviewed commit SHA was approved before any privileged host accepts artifact inputs — a signed release ticket, not a secret and never the CI run itself. + +Host-group roles (`deployment`, `persistent-testing`, `image-build`) are logical inventory labels only; core ci-fleet #22 and #23 own the executable privileged-role installers, so this template deliberately does not define deployer or tester runtime fields yet. The validator enforces the isolation that exists today: ordinary-CI routing labels must be unique across pools and must not collide with a privileged host group's name or role, environments may target only deployment-role host groups, and production credentials or authority are never granted to ordinary CI. The initializer refuses to replace a configured file unless `--force` is explicit. Run `./scripts/init.sh --help` for repository, registry, runner-group, controller, location, capacity, resource, and output options. @@ -113,8 +122,9 @@ Deleting one generic controller must not require application workflow changes. L - Every matrix job has a five-minute hard timeout, while expected test payload targets four minutes or less to reserve startup and reporting time. - Application workflows submit all independent jobs; infrastructure configuration alone controls worker capacity. - A GitHub runner group is assigned to exactly one runner pool. -- CI runner pools and deployment host groups are separate trust roles. -- Production deployment is manual and requires GitHub Environment approval. +- CI runner pools and deployment host groups are separate trust roles. Host-group `role` values (`deployment`, `persistent-testing`, `image-build`) are logical inventory labels; ordinary-CI routing labels must never collide with a privileged host group's name or role, and the executable privileged-role boundary lands with core ci-fleet #22/#23. +- Production deployment is manual and gated by a real approval outside the requesting CI identity. The gate is either a protected GitHub Environment (available only when `organization.github_plan` is `team` or `enterprise`) or an explicit `manual-external` exact-head approval recorded in `approval_evidence`. On GitHub Free, private repositories have **no** protected Environments and no Environment secrets — the template rejects any configuration claiming otherwise. +- Approval evidence records where an exact reviewed commit was approved (for example, a signed release ticket). It never contains secret values, and it never names ordinary-CI state as its own approval. - Controller engine revisions, reusable workflows, and third-party actions are pinned to immutable commits. - Configuration contains logical identifiers only. Secret values, private host details, and credentials never enter Git. - Promoted artifacts are container image digests; production does not rebuild a different image. diff --git a/examples/multi-host/fleet.json b/examples/multi-host/fleet.json index e9f071f..5aabf17 100644 --- a/examples/multi-host/fleet.json +++ b/examples/multi-host/fleet.json @@ -5,7 +5,8 @@ "slug": "sample-company", "registry": "ghcr.io/sample-company", "delivery_engine": "RandomDevelopment/ci-fleet", - "workflow_ref_policy": "immutable-commit" + "workflow_ref_policy": "immutable-commit", + "github_plan": "team" }, "runner_pools": { "trusted-ci": { @@ -59,6 +60,10 @@ "production-primary": { "role": "deployment", "environment_class": "production" + }, + "image-build-shared": { + "role": "image-build", + "environment_class": "staging" } }, "environments": { @@ -66,18 +71,21 @@ "host_group": "development-east", "automatic": true, "requires_approval": false, + "approval_mechanism": "github-environment", "required_secret_names": ["DEPLOY_AUTH"] }, "staging": { "host_group": "staging-east", "automatic": false, "requires_approval": true, + "approval_mechanism": "github-environment", "required_secret_names": ["DEPLOY_AUTH"] }, "production": { "host_group": "production-primary", "automatic": false, "requires_approval": true, + "approval_mechanism": "github-environment", "required_secret_names": ["DEPLOY_AUTH"] } }, diff --git a/fleet.json b/fleet.json index 198456c..35ec414 100644 --- a/fleet.json +++ b/fleet.json @@ -5,7 +5,8 @@ "slug": "example-org", "registry": "ghcr.io/example-org", "delivery_engine": "RandomDevelopment/ci-fleet", - "workflow_ref_policy": "immutable-commit" + "workflow_ref_policy": "immutable-commit", + "github_plan": "free" }, "runner_pools": { "trusted-ci": { @@ -41,6 +42,10 @@ "production-apps": { "role": "deployment", "environment_class": "production" + }, + "persistent-test-apps": { + "role": "persistent-testing", + "environment_class": "staging" } }, "environments": { @@ -48,12 +53,15 @@ "host_group": "development-apps", "automatic": true, "requires_approval": false, + "approval_mechanism": "manual-external", "required_secret_names": ["DEPLOY_AUTH"] }, "production": { "host_group": "production-apps", "automatic": false, "requires_approval": true, + "approval_mechanism": "manual-external", + "approval_evidence": "signed release ticket recording the exact reviewed commit SHA before any privileged host accepts artifact inputs", "required_secret_names": ["DEPLOY_AUTH"] } }, diff --git a/fleet.schema.json b/fleet.schema.json index 606536a..7020e05 100644 --- a/fleet.schema.json +++ b/fleet.schema.json @@ -16,7 +16,11 @@ "slug": {"type": "string", "pattern": "^[a-z0-9][a-z0-9-]{0,38}$"}, "registry": {"type": "string", "pattern": "^[a-z0-9.-]+/[a-z0-9._/-]+$"}, "delivery_engine": {"const": "RandomDevelopment/ci-fleet"}, - "workflow_ref_policy": {"const": "immutable-commit"} + "workflow_ref_policy": {"const": "immutable-commit"}, + "github_plan": { + "description": "GitHub plan capability class. Omitted means free: protected Environments are unavailable for private repositories and approval_mechanism must be manual-external.", + "enum": ["free", "team", "enterprise"] + } } }, "runner_pools": { @@ -97,18 +101,29 @@ "additionalProperties": false, "required": ["role", "environment_class"], "properties": { - "role": {"const": "deployment"}, + "role": { + "description": "Logical inventory label only. Ordinary CI runner pools must never carry a deployment, persistent-testing, or image-build role; the executable role boundary is owned by core ci-fleet #22/#23.", + "enum": ["deployment", "persistent-testing", "image-build"] + }, "environment_class": {"enum": ["development", "staging", "production"]} } }, "environment": { "type": "object", "additionalProperties": false, - "required": ["host_group", "automatic", "requires_approval", "required_secret_names"], + "required": ["host_group", "automatic", "requires_approval", "approval_mechanism", "required_secret_names"], "properties": { "host_group": {"type": "string"}, "automatic": {"type": "boolean"}, "requires_approval": {"type": "boolean"}, + "approval_mechanism": { + "description": "github-environment requires an organization.github_plan that actually provides protected Environments; manual-external records exact-head approval evidence outside GitHub.", + "enum": ["github-environment", "manual-external"] + }, + "approval_evidence": { + "description": "For manual-external with requires_approval true: where the exact-head approval is recorded (logical reference only, never secret values).", + "type": "string" + }, "required_secret_names": {"type": "array", "uniqueItems": true, "items": {"type": "string", "pattern": "^[A-Z][A-Z0-9_]*$"}} } }, diff --git a/scripts/init.py b/scripts/init.py index 3d3ac48..1bc3c49 100755 --- a/scripts/init.py +++ b/scripts/init.py @@ -41,6 +41,7 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--runner-cpu-cores", type=positive_integer, default=2, help="CPU cores available to each runner") parser.add_argument("--runner-memory-mib", type=positive_integer, default=4096, help="memory available to each runner") parser.add_argument("--engine-ref", required=True, help="reviewed full ci-fleet commit SHA") + parser.add_argument("--github-plan", choices=("free", "team", "enterprise"), default="free", help="GitHub plan capability class; free forbids protected-Environment approvals") parser.add_argument("--output", type=Path, default=ROOT / "fleet.json", help="output configuration path") parser.add_argument("--force", action="store_true", help="replace an existing non-example output file") return parser.parse_args() @@ -69,6 +70,7 @@ def main() -> int: fail("--max-runners must not exceed --capacity-budget") if args.runner_memory_mib < 512: fail("--runner-memory-mib must be at least 512") + approval_mechanism = "github-environment" if args.github_plan in {"team", "enterprise"} else "manual-external" repository = args.repository or f"{args.organization}/{args.project}" registry = (args.registry or f"ghcr.io/{args.organization}").rstrip("/") @@ -90,6 +92,7 @@ def main() -> int: "registry": registry, "delivery_engine": "RandomDevelopment/ci-fleet", "workflow_ref_policy": "immutable-commit", + "github_plan": args.github_plan, }, "runner_pools": { "trusted-ci": { @@ -126,12 +129,16 @@ def main() -> int: "host_group": "development-apps", "automatic": True, "requires_approval": False, + "approval_mechanism": approval_mechanism, + **({"approval_evidence": "signed release ticket recording the exact reviewed commit SHA"} if approval_mechanism == "manual-external" else {}), "required_secret_names": ["DEPLOY_AUTH"], }, "production": { "host_group": "production-apps", "automatic": False, "requires_approval": True, + "approval_mechanism": approval_mechanism, + **({"approval_evidence": "signed release ticket recording the exact reviewed commit SHA"} if approval_mechanism == "manual-external" else {}), "required_secret_names": ["DEPLOY_AUTH"], }, }, diff --git a/scripts/test_policy.py b/scripts/test_policy.py index 42493dc..44859cc 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -429,5 +429,117 @@ def test_updating_guide_preserves_adopter_state_before_commit(self) -> None: self.assertNotIn("git fetch template '+refs/tags/", guide) +class CapabilityAwarePolicyTests(unittest.TestCase): + """Issue #11: deployment policy is capability-aware and roles stay isolated.""" + + def assert_rejected(self, config: dict, expected: str, *, strict: bool = False) -> None: + errors = errors_for(config, strict=strict) + self.assertTrue(any(expected in error for error in errors), errors) + + def set_plan(self, config: dict, plan: str | None) -> None: + if plan is None: + config["organization"].pop("github_plan", None) + else: + config["organization"]["github_plan"] = plan + + def test_reference_configuration_is_valid(self) -> None: + self.assertEqual(errors_for(reference_config()), []) + + def test_multi_host_configuration_is_valid(self) -> None: + config = json.loads((ROOT / "examples" / "multi-host" / "fleet.json").read_text(encoding="utf-8")) + self.assertEqual(errors_for(config), []) + + def test_omitted_github_plan_defaults_to_free(self) -> None: + self.set_plan(reference_config(), None) + + def test_environment_approval_overclaim_on_free_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + self.set_plan(config, "free") + config["environments"]["production"]["approval_mechanism"] = "github-environment" + self.assert_rejected(config, "GitHub Free private repositories have no protected Environments") + + def test_environment_approval_requires_declared_capability(self) -> None: + config = copy.deepcopy(reference_config()) + self.set_plan(config, None) + config["environments"]["development"]["approval_mechanism"] = "github-environment" + self.assert_rejected(config, "requires organization.github_plan team or enterprise") + + def test_team_plan_supports_environment_approval(self) -> None: + config = copy.deepcopy(reference_config()) + self.set_plan(config, "team") + for environment in config["environments"].values(): + environment["approval_mechanism"] = "github-environment" + environment.pop("approval_evidence", None) + self.assertEqual(errors_for(config), []) + + def test_invalid_github_plan_is_rejected(self) -> None: + self.assert_rejected(self.with_plan(reference_config(), "unlimited"), "must be free, team, or enterprise") + + def with_plan(self, config: dict, plan: str) -> dict: + config = copy.deepcopy(config) + self.set_plan(config, plan) + return config + + def test_manual_production_without_recorded_evidence_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + config["environments"]["production"].pop("approval_evidence") + self.assert_rejected(config, "manual-external approval must record where the exact-head approval is kept") + + def test_self_approved_production_is_rejected(self) -> None: + # A production declaration whose only approval record lives inside the + # same unprivileged CI identity that requests deployment is a + # self-approval: the evidence reference must not name ordinary-CI state. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = "trusted-ci runner group job log" + self.assert_rejected(config, "must not name ordinary-CI state") + + def test_missing_approval_mechanism_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + config["environments"]["development"].pop("approval_mechanism") + self.assert_rejected(config, "missing keys") + + def test_environment_may_not_target_non_deployment_host_group(self) -> None: + config = copy.deepcopy(reference_config()) + config["host_groups"]["persistent-test-apps"]["role"] = "image-build" + config["environments"]["staging"] = { + "host_group": "persistent-test-apps", + "automatic": True, + "requires_approval": False, + "approval_mechanism": "manual-external", + "required_secret_names": [], + } + self.assert_rejected(config, "deploy only from deployment hosts") + + def test_privileged_role_kinds_are_recognized_inventory(self) -> None: + config = copy.deepcopy(reference_config()) + config["host_groups"]["persistent-test-apps"]["role"] = "persistent-testing" + self.assertEqual(errors_for(config), []) + config["host_groups"]["persistent-test-apps"]["role"] = "image-build" + self.assertEqual(errors_for(config), []) + + def test_unknown_host_group_role_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + config["host_groups"]["persistent-test-apps"]["role"] = "super-deployer" + self.assert_rejected(config, "must be deployment, persistent-testing, or image-build") + + def test_ci_label_colliding_with_privileged_group_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + config["runner_pools"]["trusted-ci"]["routing_labels"] = ["persistent-test-apps"] + self.assert_rejected(config, "privileged host-group identity") + + def test_ci_label_equal_to_role_word_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + config["runner_pools"]["trusted-ci"]["routing_labels"] = ["image-build"] + self.assert_rejected(config, "privileged host-group identity") + + def test_duplicate_routing_label_across_pools_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + duplicate = copy.deepcopy(config["runner_pools"]["trusted-ci"]) + duplicate["runner_group"] = "other-group" + duplicate["routing_labels"] = list(duplicate["routing_labels"]) + config["runner_pools"]["second-pool"] = duplicate + self.assert_rejected(config, "unique across pools") + + if __name__ == "__main__": unittest.main() diff --git a/scripts/validate.py b/scripts/validate.py index 77cd706..f8fdf92 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -190,7 +190,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: organization = config.get("organization") organization_keys = {"slug", "registry", "delivery_engine", "workflow_ref_policy"} - if validation.exact_keys(organization, "$.organization", organization_keys): + if validation.exact_keys(organization, "$.organization", organization_keys, {"github_plan"}): slug = organization.get("slug") registry = organization.get("registry") engine = organization.get("delivery_engine") @@ -199,6 +199,8 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(isinstance(engine, str) and bool(REPOSITORY.fullmatch(engine)), "$.organization.delivery_engine", "must be an owner/repository name") validation.require(engine == "RandomDevelopment/ci-fleet", "$.organization.delivery_engine", "must use the fixed reviewed public engine repository") validation.require(organization.get("workflow_ref_policy") == "immutable-commit", "$.organization.workflow_ref_policy", "must equal immutable-commit") + plan = organization.get("github_plan") + validation.require(plan is None or plan in {"free", "team", "enterprise"}, "$.organization.github_plan", "must be free, team, or enterprise; omitted means free") if strict: validation.require(slug != "example-org", "$.organization.slug", "replace the example organization before use") @@ -325,9 +327,39 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: path = f"$.host_groups.{name}" validation.require(bool(SLUG.fullmatch(name)), path, "host group name must be a lowercase slug") if validation.exact_keys(group, path, {"role", "environment_class"}): - validation.require(group.get("role") == "deployment", f"{path}.role", "must equal deployment; CI workers and deployment hosts are separate") + validation.require(group.get("role") in {"deployment", "persistent-testing", "image-build"}, f"{path}.role", "must be deployment, persistent-testing, or image-build; ordinary CI pools never carry a privileged role") validation.require(group.get("environment_class") in {"development", "staging", "production"}, f"{path}.environment_class", "must be development, staging, or production") + # Ordinary-CI routing labels are the only way a project workflow selects its + # runners; a privileged host group reusing one would let unprivileged jobs + # route onto privileged hosts. JSON Schema cannot compare across objects. + privileged_identities = {role for role in ("persistent-testing", "image-build")} + privileged_identities.update( + name + for name, group in groups.items() + if isinstance(group, dict) and group.get("role") != "deployment" + ) + ci_labels: dict[str, str] = {} + for pool_name, pool in pools.items(): + labels = pool.get("routing_labels") if isinstance(pool, dict) else None + if not isinstance(labels, list): + continue + for index, label in enumerate(labels): + if isinstance(label, str): + other = ci_labels.get(label) + validation.require( + other is None, + f"$.runner_pools.{pool_name}.routing_labels[{index}]", + f"must be unique across pools; also used by {other}" if other else "must be unique across pools", + ) + if other is None: + ci_labels[label] = pool_name + if label in privileged_identities: + validation.require(False, f"$.runner_pools.{pool_name}.routing_labels[{index}]", f"must not collide with privileged host-group identity {label}") + + plan = config.get("organization", {}).get("github_plan") if isinstance(config.get("organization"), dict) else None + environment_capable = plan in {"team", "enterprise"} + environments = config.get("environments") if not isinstance(environments, dict) or not environments: validation.errors.append("$.environments: must be a non-empty object") @@ -335,12 +367,40 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: for name, environment in environments.items(): path = f"$.environments.{name}" validation.require(bool(SLUG.fullmatch(name)), path, "environment name must be a lowercase slug") - if not validation.exact_keys(environment, path, {"host_group", "automatic", "requires_approval", "required_secret_names"}): + if not validation.exact_keys( + environment, + path, + {"host_group", "automatic", "requires_approval", "approval_mechanism", "required_secret_names"}, + {"approval_evidence"}, + ): continue host_group = environment.get("host_group") validation.require(host_group in groups, f"{path}.host_group", "must reference a declared deployment host group") + if host_group in groups and groups[host_group].get("role") != "deployment": + validation.require(False, f"{path}.host_group", "must reference a deployment-role host group; environments deploy only from deployment hosts") validation.require(type(environment.get("automatic")) is bool, f"{path}.automatic", "must be a boolean") - validation.require(type(environment.get("requires_approval")) is bool, f"{path}.requires_approval", "must be a boolean") + requires_approval = environment.get("requires_approval") + validation.require(type(requires_approval) is bool, f"{path}.requires_approval", "must be a boolean") + mechanism = environment.get("approval_mechanism") + validation.require(mechanism in {"github-environment", "manual-external"}, f"{path}.approval_mechanism", "must be github-environment or manual-external") + evidence = environment.get("approval_evidence") + validation.require(evidence is None or (isinstance(evidence, str) and bool(evidence.strip())), f"{path}.approval_evidence", "must be a logical reference to where exact-head approval is recorded, never a secret value") + if mechanism == "github-environment" and not environment_capable: + validation.require(False, f"{path}.approval_mechanism", "github-environment approval requires organization.github_plan team or enterprise; GitHub Free private repositories have no protected Environments — use manual-external") + if mechanism == "manual-external" and requires_approval and not isinstance(evidence, str): + validation.require(False, f"{path}.approval_evidence", "manual-external approval must record where the exact-head approval is kept") + if isinstance(evidence, str): + lowered = evidence.lower() + for pool_name, pool in pools.items(): + labels = pool.get("routing_labels") if isinstance(pool, dict) else None + identities = {str(pool_name), str(pool.get("runner_group"))} + identities.update(str(label) for label in labels if isinstance(label, str)) + hit = next((identity for identity in identities if identity and identity.replace("-", " ") in lowered.replace("-", " ")), None) + validation.require( + hit is None, + f"{path}.approval_evidence", + f"must not name ordinary-CI state ({hit}) as its own approval; approval evidence lives outside the requesting CI identity", + ) names = environment.get("required_secret_names") validation.require(isinstance(names, list), f"{path}.required_secret_names", "must be a list") if isinstance(names, list): From 882df10d85a6f57b4ee6690608d5ecd70f19afb2 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 04:18:52 -0500 Subject: [PATCH 02/14] fix(policy): address Codex review findings on capability gates and isolation - Include deployment-role host groups (and all declared groups) in the routing-label collision set; deployment hosts are privileged too. - Gate github-environment approval on github_plan enterprise only: required reviewers on private repositories need Enterprise, not Team. - Keep schema-v3 compatible: approval_mechanism becomes optional and the validator infers the fail-closed gate from the declared plan, so legacy environments keep validating without a data migration or version bump. - Match approval-evidence identity references as bounded hyphen-normalized phrases instead of raw substrings to stop mid-word false positives. - Type-guard pools in the evidence check so malformed runner_pools entries produce structural errors instead of an AttributeError traceback. Adds regression tests for each finding; suite is now 70 tests. --- README.md | 9 +++-- examples/multi-host/fleet.json | 2 +- fleet.schema.json | 2 +- scripts/init.py | 2 +- scripts/test_policy.py | 68 ++++++++++++++++++++++++++++++++-- scripts/validate.py | 38 +++++++++++-------- 6 files changed, 94 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index a8885e5..2d465e2 100644 --- a/README.md +++ b/README.md @@ -50,10 +50,11 @@ flowchart LR ## Capability-aware deployment policy -`organization.github_plan` declares what your GitHub plan can actually provide: `free`, `team`, or `enterprise`. Omitted means `free` (fail closed). Each environment then names its real approval gate: +- `organization.github_plan` declares what your GitHub plan can actually provide: `free`, `team`, or `enterprise`. Omitted means `free` (fail closed). Each environment then names its real approval gate: -- `approval_mechanism: github-environment` requires a plan that supports protected Environments for private repositories. The validator rejects this on `free` or when the plan is undeclared. +- `approval_mechanism: github-environment` requires a plan that supports protected Environments **and required reviewers** on private repositories — that is Enterprise only. The validator rejects this on Free, Team, or an undeclared plan. - `approval_mechanism: manual-external` is the fallback for everyone else. When `requires_approval` is true, `approval_evidence` must record where the exact reviewed commit SHA was approved before any privileged host accepts artifact inputs — a signed release ticket, not a secret and never the CI run itself. +- Omitting `approval_mechanism` infers the fail-closed gate from the declared plan, so pre-existing schema-v3 environments remain valid without edits. Host-group roles (`deployment`, `persistent-testing`, `image-build`) are logical inventory labels only; core ci-fleet #22 and #23 own the executable privileged-role installers, so this template deliberately does not define deployer or tester runtime fields yet. The validator enforces the isolation that exists today: ordinary-CI routing labels must be unique across pools and must not collide with a privileged host group's name or role, environments may target only deployment-role host groups, and production credentials or authority are never granted to ordinary CI. @@ -123,8 +124,8 @@ Deleting one generic controller must not require application workflow changes. L - Application workflows submit all independent jobs; infrastructure configuration alone controls worker capacity. - A GitHub runner group is assigned to exactly one runner pool. - CI runner pools and deployment host groups are separate trust roles. Host-group `role` values (`deployment`, `persistent-testing`, `image-build`) are logical inventory labels; ordinary-CI routing labels must never collide with a privileged host group's name or role, and the executable privileged-role boundary lands with core ci-fleet #22/#23. -- Production deployment is manual and gated by a real approval outside the requesting CI identity. The gate is either a protected GitHub Environment (available only when `organization.github_plan` is `team` or `enterprise`) or an explicit `manual-external` exact-head approval recorded in `approval_evidence`. On GitHub Free, private repositories have **no** protected Environments and no Environment secrets — the template rejects any configuration claiming otherwise. -- Approval evidence records where an exact reviewed commit was approved (for example, a signed release ticket). It never contains secret values, and it never names ordinary-CI state as its own approval. +- Production deployment is manual and gated by a real approval outside the requesting CI identity. The gate is either a protected GitHub Environment with required reviewers (available only when `organization.github_plan` is `enterprise`; required reviewers on private repositories need Enterprise) or an explicit `manual-external` exact-head approval recorded in `approval_evidence`. On GitHub Free and Team, private repositories have no required-reviewer Environment gates — the template rejects any configuration claiming otherwise. +- Approval evidence records where an exact reviewed commit was approved (for example, a signed release ticket). It never contains secret values, and it never names ordinary-CI state as its own approval. Environments may omit `approval_mechanism`: the validator then infers the fail-closed gate from the declared plan, so existing schema-v3 configurations keep validating. - Controller engine revisions, reusable workflows, and third-party actions are pinned to immutable commits. - Configuration contains logical identifiers only. Secret values, private host details, and credentials never enter Git. - Promoted artifacts are container image digests; production does not rebuild a different image. diff --git a/examples/multi-host/fleet.json b/examples/multi-host/fleet.json index 5aabf17..e376f3e 100644 --- a/examples/multi-host/fleet.json +++ b/examples/multi-host/fleet.json @@ -6,7 +6,7 @@ "registry": "ghcr.io/sample-company", "delivery_engine": "RandomDevelopment/ci-fleet", "workflow_ref_policy": "immutable-commit", - "github_plan": "team" + "github_plan": "enterprise" }, "runner_pools": { "trusted-ci": { diff --git a/fleet.schema.json b/fleet.schema.json index 7020e05..50970f7 100644 --- a/fleet.schema.json +++ b/fleet.schema.json @@ -117,7 +117,7 @@ "automatic": {"type": "boolean"}, "requires_approval": {"type": "boolean"}, "approval_mechanism": { - "description": "github-environment requires an organization.github_plan that actually provides protected Environments; manual-external records exact-head approval evidence outside GitHub.", + "description": "github-environment requires organization.github_plan enterprise (required reviewers on private repositories); manual-external records exact-head approval evidence outside GitHub. Omitted infers the fail-closed mechanism for the declared plan.", "enum": ["github-environment", "manual-external"] }, "approval_evidence": { diff --git a/scripts/init.py b/scripts/init.py index 1bc3c49..24ec7b3 100755 --- a/scripts/init.py +++ b/scripts/init.py @@ -70,7 +70,7 @@ def main() -> int: fail("--max-runners must not exceed --capacity-budget") if args.runner_memory_mib < 512: fail("--runner-memory-mib must be at least 512") - approval_mechanism = "github-environment" if args.github_plan in {"team", "enterprise"} else "manual-external" + approval_mechanism = "github-environment" if args.github_plan == "enterprise" else "manual-external" repository = args.repository or f"{args.organization}/{args.project}" registry = (args.registry or f"ghcr.io/{args.organization}").rstrip("/") diff --git a/scripts/test_policy.py b/scripts/test_policy.py index 44859cc..edbd298 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -456,13 +456,13 @@ def test_environment_approval_overclaim_on_free_is_rejected(self) -> None: config = copy.deepcopy(reference_config()) self.set_plan(config, "free") config["environments"]["production"]["approval_mechanism"] = "github-environment" - self.assert_rejected(config, "GitHub Free private repositories have no protected Environments") + self.assert_rejected(config, "unavailable for private repositories on Free and Team") def test_environment_approval_requires_declared_capability(self) -> None: config = copy.deepcopy(reference_config()) self.set_plan(config, None) config["environments"]["development"]["approval_mechanism"] = "github-environment" - self.assert_rejected(config, "requires organization.github_plan team or enterprise") + self.assert_rejected(config, "requires organization.github_plan enterprise") def test_team_plan_supports_environment_approval(self) -> None: config = copy.deepcopy(reference_config()) @@ -470,6 +470,33 @@ def test_team_plan_supports_environment_approval(self) -> None: for environment in config["environments"].values(): environment["approval_mechanism"] = "github-environment" environment.pop("approval_evidence", None) + self.assert_rejected(config, "requires organization.github_plan enterprise") + + def test_enterprise_plan_supports_environment_approval(self) -> None: + config = copy.deepcopy(reference_config()) + self.set_plan(config, "enterprise") + for environment in config["environments"].values(): + environment["approval_mechanism"] = "github-environment" + environment.pop("approval_evidence", None) + self.assertEqual(errors_for(config), []) + + def test_missing_approval_mechanism_infers_fail_closed_gate(self) -> None: + # Schema-v3 compatibility (Codex finding, PR #14): legacy environments + # without approval_mechanism stay valid and infer the gate from the + # declared plan — manual-external with required evidence here. + config = copy.deepcopy(reference_config()) + self.set_plan(config, None) + config["environments"]["development"].pop("approval_mechanism") + config["environments"]["production"].pop("approval_mechanism") + config["runner_pools"]["trusted-ci"]["runner_group"] = "ci-group" + self.assertEqual(errors_for(config), []) + + def test_legacy_environment_on_capable_plan_infers_environment_gate(self) -> None: + config = copy.deepcopy(reference_config()) + self.set_plan(config, "enterprise") + for environment in config["environments"].values(): + environment.pop("approval_mechanism", None) + environment.pop("approval_evidence", None) self.assertEqual(errors_for(config), []) def test_invalid_github_plan_is_rejected(self) -> None: @@ -493,10 +520,17 @@ def test_self_approved_production_is_rejected(self) -> None: config["environments"]["production"]["approval_evidence"] = "trusted-ci runner group job log" self.assert_rejected(config, "must not name ordinary-CI state") - def test_missing_approval_mechanism_is_rejected(self) -> None: + def test_missing_approval_mechanism_stays_valid_and_infers_gate(self) -> None: + # Supersedes the original required-field test after the Codex + # schema-v3-compatibility finding: omission now infers the gate. config = copy.deepcopy(reference_config()) config["environments"]["development"].pop("approval_mechanism") - self.assert_rejected(config, "missing keys") + self.assertEqual(errors_for(config), []) + + def test_unknown_approval_mechanism_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + config["environments"]["development"]["approval_mechanism"] = "honor-system" + self.assert_rejected(config, "must be github-environment or manual-external") def test_environment_may_not_target_non_deployment_host_group(self) -> None: config = copy.deepcopy(reference_config()) @@ -527,6 +561,32 @@ def test_ci_label_colliding_with_privileged_group_is_rejected(self) -> None: config["runner_pools"]["trusted-ci"]["routing_labels"] = ["persistent-test-apps"] self.assert_rejected(config, "privileged host-group identity") + def test_ci_label_colliding_with_deployment_group_is_rejected(self) -> None: + # Codex finding (PR #14): deployment-role host groups are privileged + # relative to ordinary CI too. + config = copy.deepcopy(reference_config()) + config["runner_pools"]["trusted-ci"]["routing_labels"] = ["development-apps"] + self.assert_rejected(config, "privileged host-group identity") + + def test_evidence_word_overlap_is_conservative_but_bounded(self) -> None: + # Codex finding (PR #14) partially applied: matching is bounded to + # hyphen-normalized whole phrases, so substrings inside larger words + # no longer false-positive... + config = copy.deepcopy(reference_config()) + config["runner_pools"]["trusted-ci"]["runner_group"] = "trusted-ci" + config["environments"]["production"]["approval_evidence"] = "untrusted-city archive of signed approvals" + self.assertEqual(errors_for(config), []) + # ...but an exact reference to the CI identity still fails closed. + config["environments"]["production"]["approval_evidence"] = "trusted-ci job log" + self.assert_rejected(config, "must not name ordinary-CI state") + + def test_malformed_pool_does_not_crash_evidence_check(self) -> None: + # Codex finding (PR #14): a non-object pool entry is reported as a + # structural error without an AttributeError traceback. + config = copy.deepcopy(reference_config()) + config["runner_pools"]["broken-pool"] = ["not-an-object"] + self.assert_rejected(config, "must be an object") + def test_ci_label_equal_to_role_word_is_rejected(self) -> None: config = copy.deepcopy(reference_config()) config["runner_pools"]["trusted-ci"]["routing_labels"] = ["image-build"] diff --git a/scripts/validate.py b/scripts/validate.py index f8fdf92..01414ef 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -333,12 +333,10 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: # Ordinary-CI routing labels are the only way a project workflow selects its # runners; a privileged host group reusing one would let unprivileged jobs # route onto privileged hosts. JSON Schema cannot compare across objects. - privileged_identities = {role for role in ("persistent-testing", "image-build")} - privileged_identities.update( - name - for name, group in groups.items() - if isinstance(group, dict) and group.get("role") != "deployment" - ) + # Every declared host group is privileged relative to ordinary CI, + # deployment groups included. + privileged_identities = {role for role in ("deployment", "persistent-testing", "image-build")} + privileged_identities.update(name for name, group in groups.items() if isinstance(group, dict)) ci_labels: dict[str, str] = {} for pool_name, pool in pools.items(): labels = pool.get("routing_labels") if isinstance(pool, dict) else None @@ -358,7 +356,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(False, f"$.runner_pools.{pool_name}.routing_labels[{index}]", f"must not collide with privileged host-group identity {label}") plan = config.get("organization", {}).get("github_plan") if isinstance(config.get("organization"), dict) else None - environment_capable = plan in {"team", "enterprise"} + environment_capable = plan == "enterprise" environments = config.get("environments") if not isinstance(environments, dict) or not environments: @@ -370,8 +368,8 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if not validation.exact_keys( environment, path, - {"host_group", "automatic", "requires_approval", "approval_mechanism", "required_secret_names"}, - {"approval_evidence"}, + {"host_group", "automatic", "requires_approval", "required_secret_names"}, + {"approval_mechanism", "approval_evidence"}, ): continue host_group = environment.get("host_group") @@ -382,20 +380,28 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: requires_approval = environment.get("requires_approval") validation.require(type(requires_approval) is bool, f"{path}.requires_approval", "must be a boolean") mechanism = environment.get("approval_mechanism") - validation.require(mechanism in {"github-environment", "manual-external"}, f"{path}.approval_mechanism", "must be github-environment or manual-external") + validation.require(mechanism is None or mechanism in {"github-environment", "manual-external"}, f"{path}.approval_mechanism", "must be github-environment or manual-external") + if mechanism is None: + # Schema-v3 compatibility: absent approval_mechanism infers the + # fail-closed gate instead of rejecting every existing adopter + # configuration (Codex finding, PR #14). + mechanism = "github-environment" if environment_capable else "manual-external" evidence = environment.get("approval_evidence") validation.require(evidence is None or (isinstance(evidence, str) and bool(evidence.strip())), f"{path}.approval_evidence", "must be a logical reference to where exact-head approval is recorded, never a secret value") if mechanism == "github-environment" and not environment_capable: - validation.require(False, f"{path}.approval_mechanism", "github-environment approval requires organization.github_plan team or enterprise; GitHub Free private repositories have no protected Environments — use manual-external") + validation.require(False, f"{path}.approval_mechanism", "github-environment required-reviewer approval requires organization.github_plan enterprise; protected Environments and required reviewers are unavailable for private repositories on Free and Team — use manual-external") if mechanism == "manual-external" and requires_approval and not isinstance(evidence, str): validation.require(False, f"{path}.approval_evidence", "manual-external approval must record where the exact-head approval is kept") if isinstance(evidence, str): - lowered = evidence.lower() + lowered = evidence.lower().replace("-", " ") for pool_name, pool in pools.items(): - labels = pool.get("routing_labels") if isinstance(pool, dict) else None - identities = {str(pool_name), str(pool.get("runner_group"))} - identities.update(str(label) for label in labels if isinstance(label, str)) - hit = next((identity for identity in identities if identity and identity.replace("-", " ") in lowered.replace("-", " ")), None) + if not isinstance(pool, dict): + continue + identities = {str(pool_name), str(pool.get("runner_group") or "")} + labels = pool.get("routing_labels") + if isinstance(labels, list): + identities.update(str(label) for label in labels if isinstance(label, str)) + hit = next((identity for identity in identities if identity and f" {identity.replace('-', ' ')} " in f" {lowered} "), None) validation.require( hit is None, f"{path}.approval_evidence", From 0f37d36d5af5d3fa9803d011c02ff6d9065f0f7d Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 04:59:15 -0500 Subject: [PATCH 03/14] fix(policy): close Codex round-2 findings on evidence matching, schema optionality, and enum guards - approval_evidence self-approval check now tokenizes on every punctuation separator and fails closed when the CI identity is named beside run-output context (trusted-ci/job-log, trusted-ci: job log), while generic prose reuse of one identity word (the initializer's 'release ticket' with --runner-label release) stays valid, so init.sh no longer crashes for such options (Codex PR #14 threads). - fleet.schema.json no longer requires approval_mechanism, matching the semantic validator's schema-v3-compatible omission inference. - enum-valued fields (github_plan, controller state/lifecycle, host-group role/environment_class, approval_mechanism) are type-checked before set membership so malformed arrays/objects yield structural errors, not unhashable-type tracebacks. --- fleet.schema.json | 4 +-- scripts/test_policy.py | 44 +++++++++++++++++++++++++++++ scripts/validate.py | 64 ++++++++++++++++++++++++++++++++++++------ 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/fleet.schema.json b/fleet.schema.json index 50970f7..d436e25 100644 --- a/fleet.schema.json +++ b/fleet.schema.json @@ -111,13 +111,13 @@ "environment": { "type": "object", "additionalProperties": false, - "required": ["host_group", "automatic", "requires_approval", "approval_mechanism", "required_secret_names"], + "required": ["host_group", "automatic", "requires_approval", "required_secret_names"], "properties": { "host_group": {"type": "string"}, "automatic": {"type": "boolean"}, "requires_approval": {"type": "boolean"}, "approval_mechanism": { - "description": "github-environment requires organization.github_plan enterprise (required reviewers on private repositories); manual-external records exact-head approval evidence outside GitHub. Omitted infers the fail-closed mechanism for the declared plan.", + "description": "github-environment requires organization.github_plan enterprise (required reviewers on private repositories); manual-external records exact-head approval evidence outside GitHub. Omitted (valid for schema-v3 compatibility) infers the fail-closed mechanism for the declared plan.", "enum": ["github-environment", "manual-external"] }, "approval_evidence": { diff --git a/scripts/test_policy.py b/scripts/test_policy.py index edbd298..a2db36d 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -499,6 +499,50 @@ def test_legacy_environment_on_capable_plan_infers_environment_gate(self) -> Non environment.pop("approval_evidence", None) self.assertEqual(errors_for(config), []) + def test_schema_keeps_approval_mechanism_optional(self) -> None: + # Codex finding (PR #14): standards-compliant JSON Schema tools must + # accept omission exactly like validate_config does, so legacy v3 + # environments are not rejected by editors. + self.assertNotIn("approval_mechanism", contract_schema()["$defs"]["environment"]["required"]) + + def test_evidence_identity_beside_punctuation_is_rejected(self) -> None: + # Codex finding (PR #14): separators other than space or hyphen must + # not let approval evidence name the requesting CI identity. + config = copy.deepcopy(reference_config()) + for evidence in ("trusted-ci/job-log", "trusted-ci: job log", "log output of run trusted-ci"): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not name ordinary-CI state") + + def test_prose_mention_of_a_generic_label_without_run_context_passes(self) -> None: + # Codex finding (PR #14): the initializer's own evidence text must + # stay valid when an operator picks a generic option value such as + # `--runner-label release`; prose reuse of one identity word is not + # self-approval, naming the CI run beside run-output context is. + config = copy.deepcopy(reference_config()) + config["runner_pools"]["trusted-ci"]["routing_labels"] = ["release"] + config["environments"]["production"]["approval_evidence"] = ( + "signed release ticket recording the exact reviewed commit SHA " + "before any privileged host accepts artifact inputs" + ) + self.assertEqual(errors_for(config), []) + + def test_non_string_enum_values_fail_closed_without_traceback(self) -> None: + # Codex finding (PR #14): malformed arrays/objects in enum-valued + # fields must yield structural errors, not unhashable-type tracebacks. + mutations = ( + lambda c: c["organization"].__setitem__("github_plan", ["free"]), + lambda c: c["controllers"]["example-ci-01"].__setitem__("state", {"active": True}), + lambda c: c["controllers"]["example-ci-01"].__setitem__("lifecycle", ["stable"]), + lambda c: c["host_groups"]["development-apps"].__setitem__("role", ["deployment"]), + lambda c: c["host_groups"]["development-apps"].__setitem__("environment_class", ["development"]), + lambda c: c["environments"]["development"].__setitem__("approval_mechanism", ["manual-external"]), + ) + for mutate in mutations: + with self.subTest(): + config = copy.deepcopy(reference_config()) + mutate(config) + self.assertTrue(errors_for(config), "expected a structural rejection") + def test_invalid_github_plan_is_rejected(self) -> None: self.assert_rejected(self.with_plan(reference_config(), "unlimited"), "must be free, team, or enterprise") diff --git a/scripts/validate.py b/scripts/validate.py index 01414ef..e2e7491 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -173,6 +173,49 @@ def scan_tree_path_list(path_list: Path, validation: Validation) -> None: validation.errors.append(f"{relative_text}: secret-bearing files are forbidden") +def evidence_tokens(text: str) -> list[str]: + """Tokenize approval evidence, splitting on every non-token separator. + + Punctuation must not let evidence name a CI identity beside run-output + context (``trusted-ci/job-log``, ``trusted-ci: job log``), so any + character that is not a slug token character acts as a delimiter and + hyphens inside tokens are normalized to spaces. + """ + return [ + re.sub(r"[-_]+", " ", chunk).strip() + for chunk in re.split(r"[^0-9a-z]+", text.lower()) + if chunk + ] + + +# ponytail: keyword-window self-approval heuristic — it fails closed on +# evidence that mentions CI state near run-output words, but cannot parse +# meaning; replace with structured evidence fields (type + locator) if +# false positives ever matter. +EVIDENCE_RUN_OUTPUT_MARKERS = frozenset({ + "artifact", "artifacts", "console", "job", "jobs", "log", "logs", + "output", "run", "runner", "runners", "runs", "workflow", +}) + + +def evidence_names_ci_state(evidence: str, identity: str, window: int = 3) -> bool: + """True when the evidence points at this CI identity's own run output. + + A bare prose reuse of one identity word (a ``release`` ticket when the + routing label happens to be ``release``) is not self-approval; naming + the CI run or its logs beside the identity is. + """ + tokens = evidence_tokens(evidence) + wanted = set(evidence_tokens(identity)) + last = len(tokens) - 1 + for index, token in enumerate(tokens): + if token in wanted: + nearby = tokens[max(0, index - window):min(last, index + window) + 1] + if wanted.isdisjoint(EVIDENCE_RUN_OUTPUT_MARKERS) and set(nearby) & EVIDENCE_RUN_OUTPUT_MARKERS: + return True + return False + + def validate_config(config: Any, validation: Validation, strict: bool) -> None: required_top = { "schema_version", @@ -200,7 +243,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(engine == "RandomDevelopment/ci-fleet", "$.organization.delivery_engine", "must use the fixed reviewed public engine repository") validation.require(organization.get("workflow_ref_policy") == "immutable-commit", "$.organization.workflow_ref_policy", "must equal immutable-commit") plan = organization.get("github_plan") - validation.require(plan is None or plan in {"free", "team", "enterprise"}, "$.organization.github_plan", "must be free, team, or enterprise; omitted means free") + validation.require(plan is None or (isinstance(plan, str) and plan in {"free", "team", "enterprise"}), "$.organization.github_plan", "must be free, team, or enterprise; omitted means free") if strict: validation.require(slug != "example-org", "$.organization.slug", "replace the example organization before use") @@ -282,7 +325,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: maximum = controller.get("max_runners") validation.require(isinstance(pool_name, str) and pool_name in pools, f"{path}.pool", "must reference a declared runner pool") validation.require(isinstance(location, str) and bool(SLUG.fullmatch(location)), f"{path}.location", "must be a logical location slug, never an address") - validation.require(state in {"active", "drained", "disabled"}, f"{path}.state", "must be active, drained, or disabled") + validation.require(state in {"active", "drained", "disabled"} if isinstance(state, str) else False, f"{path}.state", "must be active, drained, or disabled") validation.require(isinstance(scale_set, str) and bool(SLUG.fullmatch(scale_set)), f"{path}.scale_set_name", "must be a lowercase scale-set slug") if isinstance(scale_set, str) and isinstance(name, str): validation.require(name in scale_set, f"{path}.scale_set_name", "must include the controller ID required by managed preflight") @@ -291,7 +334,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.errors.append(f"{path}.scale_set_name: must be unique; also used by {scale_sets[scale_set]}") else: scale_sets[scale_set] = name - validation.require(lifecycle in {"experimental", "stable", "retiring"}, f"{path}.lifecycle", "must be experimental, stable, or retiring") + validation.require(lifecycle in {"experimental", "stable", "retiring"} if isinstance(lifecycle, str) else False, f"{path}.lifecycle", "must be experimental, stable, or retiring") validation.require(isinstance(engine_ref, str) and bool(COMMIT_SHA.fullmatch(engine_ref)) and engine_ref != "0" * 40, f"{path}.engine_ref", "must be a nonzero full lowercase commit SHA") validation.require(type(minimum) is int and minimum >= 0, f"{path}.min_runners", "must be a non-negative integer") validation.require(type(maximum) is int and maximum > 0, f"{path}.max_runners", "must be a positive integer") @@ -327,8 +370,10 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: path = f"$.host_groups.{name}" validation.require(bool(SLUG.fullmatch(name)), path, "host group name must be a lowercase slug") if validation.exact_keys(group, path, {"role", "environment_class"}): - validation.require(group.get("role") in {"deployment", "persistent-testing", "image-build"}, f"{path}.role", "must be deployment, persistent-testing, or image-build; ordinary CI pools never carry a privileged role") - validation.require(group.get("environment_class") in {"development", "staging", "production"}, f"{path}.environment_class", "must be development, staging, or production") + role = group.get("role") + validation.require(isinstance(role, str) and role in {"deployment", "persistent-testing", "image-build"}, f"{path}.role", "must be deployment, persistent-testing, or image-build; ordinary CI pools never carry a privileged role") + environment_class = group.get("environment_class") + validation.require(isinstance(environment_class, str) and environment_class in {"development", "staging", "production"}, f"{path}.environment_class", "must be development, staging, or production") # Ordinary-CI routing labels are the only way a project workflow selects its # runners; a privileged host group reusing one would let unprivileged jobs @@ -380,7 +425,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: requires_approval = environment.get("requires_approval") validation.require(type(requires_approval) is bool, f"{path}.requires_approval", "must be a boolean") mechanism = environment.get("approval_mechanism") - validation.require(mechanism is None or mechanism in {"github-environment", "manual-external"}, f"{path}.approval_mechanism", "must be github-environment or manual-external") + validation.require(mechanism is None or (isinstance(mechanism, str) and mechanism in {"github-environment", "manual-external"}), f"{path}.approval_mechanism", "must be github-environment or manual-external") if mechanism is None: # Schema-v3 compatibility: absent approval_mechanism infers the # fail-closed gate instead of rejecting every existing adopter @@ -393,7 +438,10 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if mechanism == "manual-external" and requires_approval and not isinstance(evidence, str): validation.require(False, f"{path}.approval_evidence", "manual-external approval must record where the exact-head approval is kept") if isinstance(evidence, str): - lowered = evidence.lower().replace("-", " ") + # Token comparison with a run-output window: punctuation cannot + # hide the CI identity ("trusted-ci/job-log", "trusted-ci: job + # log" both fail), but prose that merely reuses one generic word + # (the initializer's "release ticket") stays valid. for pool_name, pool in pools.items(): if not isinstance(pool, dict): continue @@ -401,7 +449,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: labels = pool.get("routing_labels") if isinstance(labels, list): identities.update(str(label) for label in labels if isinstance(label, str)) - hit = next((identity for identity in identities if identity and f" {identity.replace('-', ' ')} " in f" {lowered} "), None) + hit = next((identity for identity in identities if evidence_names_ci_state(evidence, identity)), None) validation.require( hit is None, f"{path}.approval_evidence", From ccbcd83a683aa30e5c14678fc78dab36111bdbd5 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 05:31:06 -0500 Subject: [PATCH 04/14] fix(policy): preserve legacy v3 evidence omission, marker-word identities, and initializer approval locators Codex review round 3 (PR #14): - the actual pre-branch schema-v3 fleet.json omitted both approval_mechanism and approval_evidence; non-strict validation keeps that import contract while --strict still demands a locator before use, pinned by a regression test on the real e483998 fleet.json; - a CI identity that is itself a run-output word (run, job, workflow) no longer disables self-approval detection via the disjointness shortcut: exact token matches count for such identities; - init.sh writes a REPLACE-ME placeholder that --strict rejects until the operator passes --approval-evidence with a real locator, instead of a generic prose sentence strict mode silently accepted. --- scripts/init.py | 9 ++++++++- scripts/test_policy.py | 40 ++++++++++++++++++++++++++++++++++++++-- scripts/validate.py | 25 ++++++++++++++++++++++--- 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/scripts/init.py b/scripts/init.py index 24ec7b3..c8e7c5c 100755 --- a/scripts/init.py +++ b/scripts/init.py @@ -42,6 +42,7 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--runner-memory-mib", type=positive_integer, default=4096, help="memory available to each runner") parser.add_argument("--engine-ref", required=True, help="reviewed full ci-fleet commit SHA") parser.add_argument("--github-plan", choices=("free", "team", "enterprise"), default="free", help="GitHub plan capability class; free forbids protected-Environment approvals") + parser.add_argument("--approval-evidence", help="where the exact-head production approval is recorded (ticket, path, or system reference); required for manual-external production so the generated configuration passes --strict") parser.add_argument("--output", type=Path, default=ROOT / "fleet.json", help="output configuration path") parser.add_argument("--force", action="store_true", help="replace an existing non-example output file") return parser.parse_args() @@ -71,6 +72,12 @@ def main() -> int: if args.runner_memory_mib < 512: fail("--runner-memory-mib must be at least 512") approval_mechanism = "github-environment" if args.github_plan == "enterprise" else "manual-external" + # A real locator must come from the operator: a generic prose sentence + # would name no actual approval record (Codex, PR #14 round 3). The + # placeholder is deliberately rejected by --strict until replaced. + production_evidence = args.approval_evidence or ( + "REPLACE-ME: record where the exact reviewed commit SHA approval is kept" + ) repository = args.repository or f"{args.organization}/{args.project}" registry = (args.registry or f"ghcr.io/{args.organization}").rstrip("/") @@ -138,7 +145,7 @@ def main() -> int: "automatic": False, "requires_approval": True, "approval_mechanism": approval_mechanism, - **({"approval_evidence": "signed release ticket recording the exact reviewed commit SHA"} if approval_mechanism == "manual-external" else {}), + **({"approval_evidence": production_evidence} if approval_mechanism == "manual-external" else {}), "required_secret_names": ["DEPLOY_AUTH"], }, }, diff --git a/scripts/test_policy.py b/scripts/test_policy.py index a2db36d..616a65c 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -526,6 +526,39 @@ def test_prose_mention_of_a_generic_label_without_run_context_passes(self) -> No ) self.assertEqual(errors_for(config), []) + def test_pre_branch_v3_configuration_without_evidence_fields_stays_valid(self) -> None: + # Codex finding, round 3 (PR #14): the actual pre-change schema-v3 + # fleet.json omitted both approval_mechanism AND approval_evidence. + # Compatibility means that exact shape keeps validating; requiring + # evidence for inferred manual-external would force adopters to edit + # data just to import the validator. + config = json.loads( + subprocess.check_output(["git", "show", "e483998:fleet.json"], text=True) + ) + self.assertEqual(errors_for(config), []) + self.assertTrue(errors_for(config, strict=True), "strict still rejects example values") + + def test_initializer_placeholder_evidence_fails_strict(self) -> None: + # Codex finding, round 3 (PR #14): the initializer's generated + # production gate must not pass --strict with a generic prose + # sentence; the operator records a real locator via + # init.sh --approval-evidence. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "REPLACE-ME: record where the exact reviewed commit SHA approval is kept" + ) + self.assertEqual(errors_for(config), []) + self.assert_rejected(config, "initializer placeholder", strict=True) + + def test_marker_word_ci_identity_still_detected_beside_run_context(self) -> None: + # Codex finding, round 3 (PR #14): a pool named like a run-output + # marker ("run", "job", "workflow") must not disable self-approval + # detection via the disjointness shortcut. + config = copy.deepcopy(reference_config()) + config["runner_pools"]["trusted-ci"]["routing_labels"] = ["run"] + config["environments"]["production"]["approval_evidence"] = "run job log" + self.assert_rejected(config, "must not name ordinary-CI state") + def test_non_string_enum_values_fail_closed_without_traceback(self) -> None: # Codex finding (PR #14): malformed arrays/objects in enum-valued # fields must yield structural errors, not unhashable-type tracebacks. @@ -551,10 +584,13 @@ def with_plan(self, config: dict, plan: str) -> dict: self.set_plan(config, plan) return config - def test_manual_production_without_recorded_evidence_is_rejected(self) -> None: + def test_manual_production_without_recorded_evidence_fails_strict_only(self) -> None: + # Round-3 compatibility: non-strict keeps the legacy v3 omission + # contract; strict mode (pre-use gate) demands the locator. config = copy.deepcopy(reference_config()) config["environments"]["production"].pop("approval_evidence") - self.assert_rejected(config, "manual-external approval must record where the exact-head approval is kept") + self.assertEqual(errors_for(config), []) + self.assert_rejected(config, "manual-external approval must record where the exact-head approval is kept", strict=True) def test_self_approved_production_is_rejected(self) -> None: # A production declaration whose only approval record lives inside the diff --git a/scripts/validate.py b/scripts/validate.py index e2e7491..25fe845 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -203,15 +203,23 @@ def evidence_names_ci_state(evidence: str, identity: str, window: int = 3) -> bo A bare prose reuse of one identity word (a ``release`` ticket when the routing label happens to be ``release``) is not self-approval; naming - the CI run or its logs beside the identity is. + the CI run or its logs beside the identity is. An identity that is + itself a run-output word (``run``, ``job``) is detected by exact token + match — the marker words must never disable detection. """ tokens = evidence_tokens(evidence) wanted = set(evidence_tokens(identity)) + if not wanted: + return False + if wanted & EVIDENCE_RUN_OUTPUT_MARKERS: + # The identity collides with run-output vocabulary: any exact + # occurrence counts, no context window can disambiguate it. + return wanted <= set(tokens) last = len(tokens) - 1 for index, token in enumerate(tokens): if token in wanted: nearby = tokens[max(0, index - window):min(last, index + window) + 1] - if wanted.isdisjoint(EVIDENCE_RUN_OUTPUT_MARKERS) and set(nearby) & EVIDENCE_RUN_OUTPUT_MARKERS: + if set(nearby) & EVIDENCE_RUN_OUTPUT_MARKERS: return True return False @@ -436,7 +444,18 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if mechanism == "github-environment" and not environment_capable: validation.require(False, f"{path}.approval_mechanism", "github-environment required-reviewer approval requires organization.github_plan enterprise; protected Environments and required reviewers are unavailable for private repositories on Free and Team — use manual-external") if mechanism == "manual-external" and requires_approval and not isinstance(evidence, str): - validation.require(False, f"{path}.approval_evidence", "manual-external approval must record where the exact-head approval is kept") + # Legacy schema-v3 environments omit approval_evidence entirely; + # rejecting them here would break adopter imports (Codex, PR #14 + # round 3). Non-strict validation keeps that contract. Strict + # mode — the gate before real use — still demands a locator. + if strict: + validation.require(False, f"{path}.approval_evidence", "manual-external approval must record where the exact-head approval is kept") + if isinstance(evidence, str) and "REPLACE-ME:" in evidence: + # The initializer's placeholder: it names no actual approval + # record, so strict mode refuses to bless it (Codex, PR #14 + # round 3). Non-strict stays silent for legacy import paths. + if strict: + validation.require(False, f"{path}.approval_evidence", "is an initializer placeholder; record a real approval locator (ticket, path, or system reference)") if isinstance(evidence, str): # Token comparison with a run-output window: punctuation cannot # hide the CI identity ("trusted-ci/job-log", "trusted-ci: job From 89ede920aaa3f87fd017637180d30678b598cc8d Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 05:34:43 -0500 Subject: [PATCH 05/14] test(ci): pass --approval-evidence in the initializer workflow step The strict placeholder rejection from ccbcd83 is intentional, so the workflow's happy-path initializer call now supplies a real approval locator and additionally proves that omitting one fails closed. --- .github/workflows/validate.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 033a91a..dd92fd7 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -86,5 +86,16 @@ jobs: --organization test-company \ --project test-app \ --engine-ref "${engine_ref}" \ + --approval-evidence "release ticket RT-1042 in the ops tracker" \ --output "${temporary_directory}/fleet.json" test "$(stat -c '%a' "${temporary_directory}/fleet.json")" = 644 + # The placeholder evidence must fail strict until replaced: + if ./scripts/init.sh \ + --organization test-company \ + --project test-app \ + --engine-ref "${engine_ref}" \ + --output "${temporary_directory}/placeholder.json"; then + echo 'initializer accepted a missing approval locator' >&2 + exit 1 + fi + test ! -e "${temporary_directory}/placeholder.json" From 3f9ec52494e34637999ea21dec5d042cebb786d1 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 06:24:00 -0500 Subject: [PATCH 06/14] fix(policy): require initializer approval locator, close evidence-bypass holes, enforce explicit manual gates in every mode Codex round-4 findings (PR #14): - marker-word CI identities no longer disable self-approval detection (complete ordered identity-phrase match replaces the disjointness shortcut and the fixed three-token window) - multi-token identities match as a whole phrase, so a single component near run-output vocabulary cannot reject legitimate external evidence - README quick-start passes --approval-evidence explicitly, keeping the documented onboarding command usable under strict validation - an explicitly selected manual-external gate requires its evidence in every validation mode; only environments omitting BOTH new fields keep the legacy schema-v3 omission contract --- README.md | 3 ++- scripts/test_policy.py | 44 +++++++++++++++++++++++++++----- scripts/validate.py | 57 ++++++++++++++++++++---------------------- 3 files changed, 67 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 2d465e2..4b5c10c 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,8 @@ flowchart LR --location primary-site \ --capacity-budget 1 \ --max-runners 1 \ - --engine-ref + --engine-ref \ + --approval-evidence "" ``` 3. Edit `fleet.json` to add the organization's real logical mappings. diff --git a/scripts/test_policy.py b/scripts/test_policy.py index 616a65c..9e79eeb 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -522,9 +522,10 @@ def test_prose_mention_of_a_generic_label_without_run_context_passes(self) -> No config["runner_pools"]["trusted-ci"]["routing_labels"] = ["release"] config["environments"]["production"]["approval_evidence"] = ( "signed release ticket recording the exact reviewed commit SHA " - "before any privileged host accepts artifact inputs" + "approved by the responsible engineer" ) self.assertEqual(errors_for(config), []) + self.assertEqual(errors_for(reference_config()), []) def test_pre_branch_v3_configuration_without_evidence_fields_stays_valid(self) -> None: # Codex finding, round 3 (PR #14): the actual pre-change schema-v3 @@ -536,7 +537,35 @@ def test_pre_branch_v3_configuration_without_evidence_fields_stays_valid(self) - subprocess.check_output(["git", "show", "e483998:fleet.json"], text=True) ) self.assertEqual(errors_for(config), []) - self.assertTrue(errors_for(config, strict=True), "strict still rejects example values") + + def test_explicit_manual_gate_requires_evidence_in_every_mode(self) -> None: + # Codex finding, round 4 (PR #14): legacy tolerance covers only + # environments omitting BOTH new fields; an explicitly selected + # manual-external gate needs its locator even non-strict. + config = copy.deepcopy(reference_config()) + env = config["environments"]["production"] + env["approval_mechanism"] = "manual-external" + env.pop("approval_evidence", None) + self.assert_rejected(config, "manual-external approval must record where the exact-head approval is kept") + + def test_identity_and_output_marker_split_by_prose_is_rejected(self) -> None: + # Codex finding, round 4 (PR #14): prose between the identity and the + # run-output marker must not defeat detection. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "trusted-ci record for the exact reviewed commit SHA in the workflow log" + ) + self.assert_rejected(config, "must not name ordinary-CI state") + + def test_partial_identity_component_near_marker_is_accepted(self) -> None: + # Codex finding, round 4 (PR #14): only the complete ordered + # identity phrase counts — one component ("trusted") near a marker + # must not reject legitimate external evidence. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "trusted release workflow ticket RT-1042" + ) + self.assertEqual(errors_for(config), []) def test_initializer_placeholder_evidence_fails_strict(self) -> None: # Codex finding, round 3 (PR #14): the initializer's generated @@ -584,11 +613,14 @@ def with_plan(self, config: dict, plan: str) -> dict: self.set_plan(config, plan) return config - def test_manual_production_without_recorded_evidence_fails_strict_only(self) -> None: - # Round-3 compatibility: non-strict keeps the legacy v3 omission - # contract; strict mode (pre-use gate) demands the locator. + def test_legacy_omission_of_both_fields_stays_valid_non_strict(self) -> None: + # Round-3 compatibility, narrowed by round 4: only environments + # omitting BOTH new fields keep the legacy v3 contract, and only in + # non-strict mode; strict mode still demands the locator. config = copy.deepcopy(reference_config()) - config["environments"]["production"].pop("approval_evidence") + env = config["environments"]["production"] + env.pop("approval_evidence") + env.pop("approval_mechanism") self.assertEqual(errors_for(config), []) self.assert_rejected(config, "manual-external approval must record where the exact-head approval is kept", strict=True) diff --git a/scripts/validate.py b/scripts/validate.py index 25fe845..08abe47 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -188,39 +188,33 @@ def evidence_tokens(text: str) -> list[str]: ] -# ponytail: keyword-window self-approval heuristic — it fails closed on -# evidence that mentions CI state near run-output words, but cannot parse +# ponytail: phrase + marker-word heuristic — it fails closed on evidence +# that names the CI identity and mentions run-output words, but cannot parse # meaning; replace with structured evidence fields (type + locator) if # false positives ever matter. EVIDENCE_RUN_OUTPUT_MARKERS = frozenset({ "artifact", "artifacts", "console", "job", "jobs", "log", "logs", - "output", "run", "runner", "runners", "runs", "workflow", + "output", "run", "runner", "runners", "runs", "stdout", }) -def evidence_names_ci_state(evidence: str, identity: str, window: int = 3) -> bool: - """True when the evidence points at this CI identity's own run output. +def evidence_names_ci_state(evidence: str, identity: str) -> bool: + """True when the evidence names the complete CI identity phrase. - A bare prose reuse of one identity word (a ``release`` ticket when the - routing label happens to be ``release``) is not self-approval; naming - the CI run or its logs beside the identity is. An identity that is - itself a run-output word (``run``, ``job``) is detected by exact token - match — the marker words must never disable detection. + The identity must appear as a contiguous ordered token phrase (its + hyphen components normalized to words) anywhere in the evidence, and + run-output vocabulary must also occur somewhere. Splitting the two by + prose does not help: evidence that names the CI workflow log as its + approval record is self-approval regardless of distance (Codex, PR #14). """ tokens = evidence_tokens(evidence) - wanted = set(evidence_tokens(identity)) + wanted = evidence_tokens(identity) if not wanted: return False - if wanted & EVIDENCE_RUN_OUTPUT_MARKERS: - # The identity collides with run-output vocabulary: any exact - # occurrence counts, no context window can disambiguate it. - return wanted <= set(tokens) - last = len(tokens) - 1 - for index, token in enumerate(tokens): - if token in wanted: - nearby = tokens[max(0, index - window):min(last, index + window) + 1] - if set(nearby) & EVIDENCE_RUN_OUTPUT_MARKERS: - return True + span = len(wanted) + for start in range(len(tokens) - span + 1): + if tokens[start:start + span] == wanted: + return bool(set(tokens) & EVIDENCE_RUN_OUTPUT_MARKERS) return False @@ -434,6 +428,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(type(requires_approval) is bool, f"{path}.requires_approval", "must be a boolean") mechanism = environment.get("approval_mechanism") validation.require(mechanism is None or (isinstance(mechanism, str) and mechanism in {"github-environment", "manual-external"}), f"{path}.approval_mechanism", "must be github-environment or manual-external") + declared_mechanism = mechanism if mechanism is None: # Schema-v3 compatibility: absent approval_mechanism infers the # fail-closed gate instead of rejecting every existing adopter @@ -444,11 +439,11 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if mechanism == "github-environment" and not environment_capable: validation.require(False, f"{path}.approval_mechanism", "github-environment required-reviewer approval requires organization.github_plan enterprise; protected Environments and required reviewers are unavailable for private repositories on Free and Team — use manual-external") if mechanism == "manual-external" and requires_approval and not isinstance(evidence, str): - # Legacy schema-v3 environments omit approval_evidence entirely; - # rejecting them here would break adopter imports (Codex, PR #14 - # round 3). Non-strict validation keeps that contract. Strict - # mode — the gate before real use — still demands a locator. - if strict: + # Legacy schema-v3 compatibility: only environments that omit + # BOTH new fields (approval_mechanism and approval_evidence) are + # tolerated without evidence; an explicitly selected manual gate + # must record its locator in every mode (Codex, PR #14 round 3). + if declared_mechanism is not None or strict: validation.require(False, f"{path}.approval_evidence", "manual-external approval must record where the exact-head approval is kept") if isinstance(evidence, str) and "REPLACE-ME:" in evidence: # The initializer's placeholder: it names no actual approval @@ -457,10 +452,12 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if strict: validation.require(False, f"{path}.approval_evidence", "is an initializer placeholder; record a real approval locator (ticket, path, or system reference)") if isinstance(evidence, str): - # Token comparison with a run-output window: punctuation cannot - # hide the CI identity ("trusted-ci/job-log", "trusted-ci: job - # log" both fail), but prose that merely reuses one generic word - # (the initializer's "release ticket") stays valid. + # Complete ordered identity-phrase match against run-output + # vocabulary: punctuation cannot hide the CI identity + # ("trusted-ci/job-log", "trusted-ci: job log" both fail), and + # neither can prose between the identity and its output marker, + # but prose that merely reuses one generic word (the + # initializer's "release ticket") stays valid. for pool_name, pool in pools.items(): if not isinstance(pool, dict): continue From 6416a5f603a9a692aff305baa5947c0c5dcdfd78 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 06:41:02 -0500 Subject: [PATCH 07/14] fix(policy): close Codex round-5 findings on controller identities, explicit nulls, and evidence locators - self-approval detection now also covers controller IDs and scale-set names: evidence citing their workflow logs is ordinary-CI self-approval - an explicitly null approval_mechanism fails closed instead of inheriting the schema-v3 legacy field-omission compatibility exception - approval_evidence locators must not contain host addresses or internal hostnames (IPv4, bracketed IPv6, >=3-label hostnames, private-TLD suffixes), keeping infrastructure details out of the Git-authored configuration per AGENTS.md --- scripts/test_policy.py | 32 ++++++++++++++++++++++++++++++++ scripts/validate.py | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/scripts/test_policy.py b/scripts/test_policy.py index 9e79eeb..34a5fb2 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -579,6 +579,38 @@ def test_initializer_placeholder_evidence_fails_strict(self) -> None: self.assertEqual(errors_for(config), []) self.assert_rejected(config, "initializer placeholder", strict=True) + def test_controller_identity_in_evidence_is_rejected(self) -> None: + # Codex finding, round 5 (PR #14): controller IDs and scale-set names + # are ordinary-CI identities too — evidence citing their run output + # is self-approval. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "example-ci-01 workflow job log for exact reviewed commit" + ) + self.assert_rejected(config, "must not name ordinary-CI state") + + def test_explicit_null_approval_mechanism_is_rejected(self) -> None: + # Codex finding, round 5 (PR #14): an explicit null is not legacy + # field omission; it must fail closed instead of inheriting the + # schema-v3 compatibility exception. + config = copy.deepcopy(reference_config()) + env = config["environments"]["production"] + env["approval_mechanism"] = None + env.pop("approval_evidence") + self.assert_rejected(config, "must be github-environment or manual-external") + + def test_host_address_in_approval_evidence_is_rejected(self) -> None: + # Codex finding, round 5 (PR #14): the free-form evidence locator + # must not become a channel for private infrastructure details; + # host addresses and hostnames are forbidden by AGENTS.md. + config = copy.deepcopy(reference_config()) + for evidence in ( + "approval record at https://10.0.0.12/tickets/RT-1042", + "approval recorded on ci-runner-01.internal", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not contain host addresses or internal hostnames") + def test_marker_word_ci_identity_still_detected_beside_run_context(self) -> None: # Codex finding, round 3 (PR #14): a pool named like a run-output # marker ("run", "job", "workflow") must not disable self-approval diff --git a/scripts/validate.py b/scripts/validate.py index 08abe47..cf59695 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -197,6 +197,16 @@ def evidence_tokens(text: str) -> list[str]: "output", "run", "runner", "runners", "runs", "stdout", }) +# Host-local infrastructure details must never enter the Git-authored +# configuration through the free-form evidence locator (AGENTS.md). +FORBIDDEN_EVIDENCE_ADDRESS = re.compile( + r"(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}" + r"|\[[0-9a-f:]+\]" # bracketed IPv6 + r"|[a-z0-9-]+(?:\.[a-z0-9-]+){2,}" # >=3-label hostname + r"|[a-z0-9-]+(?:\.[a-z0-9-]+)*\.(?:internal|local|lan|corp|private|home|intranet)\b)", + re.IGNORECASE, +) + def evidence_names_ci_state(evidence: str, identity: str) -> bool: """True when the evidence names the complete CI identity phrase. @@ -428,7 +438,13 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(type(requires_approval) is bool, f"{path}.requires_approval", "must be a boolean") mechanism = environment.get("approval_mechanism") validation.require(mechanism is None or (isinstance(mechanism, str) and mechanism in {"github-environment", "manual-external"}), f"{path}.approval_mechanism", "must be github-environment or manual-external") - declared_mechanism = mechanism + declared_mechanism = mechanism if mechanism is not None else None + legacy_omission = "approval_mechanism" not in environment + if mechanism is None and not legacy_omission: + # An explicit null is a malformed value, not legacy field + # omission; it must not inherit the schema-v3 compatibility + # exception (Codex, PR #14 round 5). + validation.require(False, f"{path}.approval_mechanism", "must be github-environment or manual-external") if mechanism is None: # Schema-v3 compatibility: absent approval_mechanism infers the # fail-closed gate instead of rejecting every existing adopter @@ -458,6 +474,11 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: # neither can prose between the identity and its output marker, # but prose that merely reuses one generic word (the # initializer's "release ticket") stays valid. + validation.require( + FORBIDDEN_EVIDENCE_ADDRESS.search(evidence) is None, + f"{path}.approval_evidence", + "must not contain host addresses or internal hostnames; reference the approval record by ticket, path, or system name only (AGENTS.md forbids infrastructure details in configuration)", + ) for pool_name, pool in pools.items(): if not isinstance(pool, dict): continue @@ -466,6 +487,25 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if isinstance(labels, list): identities.update(str(label) for label in labels if isinstance(label, str)) hit = next((identity for identity in identities if evidence_names_ci_state(evidence, identity)), None) + if hit is None: + # Controller IDs and scale-set names are ordinary-CI + # identities too: their workflow logs are self-approval + # evidence just like pool outputs (Codex, PR #14). + controllers = config.get("controllers") + controller_ids = list(controllers) if isinstance(controllers, dict) else [] + scale_sets = [ + str(controller.get("scale_set_name")) + for controller in controllers.values() + if isinstance(controllers, dict) and isinstance(controller, dict) and controller.get("scale_set_name") + ] + hit = next( + ( + identity + for identity in (*controller_ids, *scale_sets) + if evidence_names_ci_state(evidence, identity) + ), + None, + ) validation.require( hit is None, f"{path}.approval_evidence", From 30f8c224dc109d488d91cf3257c741372ea058f5 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:51:20 -0500 Subject: [PATCH 08/14] fix(policy): close Codex round-6 findings on evidence address and credential guards Reject unbracketed IPv6 literals, type-guard malformed host groups before reading role, distinguish omitted vs explicit-null approval_evidence, extend CI execution marker vocabulary, and reject credential-bearing URI userinfo in approval evidence. --- scripts/test_policy.py | 41 ++++++++++++++++++++++++++ scripts/validate.py | 67 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/scripts/test_policy.py b/scripts/test_policy.py index 34a5fb2..a18bb8c 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -744,6 +744,47 @@ def test_duplicate_routing_label_across_pools_is_rejected(self) -> None: config["runner_pools"]["second-pool"] = duplicate self.assert_rejected(config, "unique across pools") + def test_unbracketed_ipv6_in_approval_evidence_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + for evidence in ( + "approval recorded on 2001:db8::1 ticket RT-1042", + "signed off at 2001:db8:85a3::8a2e:370:7334", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not contain host addresses or internal hostnames") + + def test_malformed_host_group_does_not_crash_environment_validation(self) -> None: + config = copy.deepcopy(reference_config()) + config["host_groups"]["development-apps"] = [] + self.assert_rejected(config, "must be an object") + + def test_explicit_null_approval_evidence_is_rejected_everywhere(self) -> None: + config = copy.deepcopy(reference_config()) + for env_name in ("development", "production"): + env = config["environments"][env_name] + env["approval_evidence"] = None + self.assert_rejected(config, "must be a logical reference to where exact-head approval is recorded") + + def test_ci_execution_markers_in_evidence_are_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + for evidence in ( + "trusted-ci workflow 123 approved exact reviewed commit SHA", + "trusted-ci workflow log", + "trusted-ci action output", + "trusted-ci check result", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not name ordinary-CI state") + + def test_credential_uri_userinfo_in_approval_evidence_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + for evidence in ( + "approval at https://reviewer:s3cr3t@example.com/RT-1042", + "approved via http://admin:password@ci-log.internal/run/1", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not contain credential-bearing URI userinfo") + if __name__ == "__main__": unittest.main() diff --git a/scripts/validate.py b/scripts/validate.py index cf59695..ac3206c 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -193,15 +193,44 @@ def evidence_tokens(text: str) -> list[str]: # meaning; replace with structured evidence fields (type + locator) if # false positives ever matter. EVIDENCE_RUN_OUTPUT_MARKERS = frozenset({ - "artifact", "artifacts", "console", "job", "jobs", "log", "logs", + "artifact", "artifacts", "action", "actions", "check", "checks", + "console", "job", "jobs", "log", "logs", "output", "run", "runner", "runners", "runs", "stdout", + "workflow", "workflows", }) +def _split_ipv6_zone(address: str) -> tuple[str, str]: + if "%25" in address: + address, zone = address.split("%25", 1) + return address, zone + if "%" in address: + address, zone = address.split("%", 1) + return address, zone + return address, "" + + +def evidence_contains_bare_ipv6(text: str) -> bool: + for token in text.split(): + if token.startswith("[") and token.endswith("]"): + continue + candidate, _ = _split_ipv6_zone(token) + if candidate.count(":") >= 2 and re.fullmatch(r"[0-9a-f:]+", candidate, re.IGNORECASE): + return True + return False + + +FORBIDDEN_CREDENTIAL_USERINFO = re.compile(r"//[^@/\s]+:[^@/\s]+@") + + +def evidence_contains_credentials(text: str) -> bool: + return bool(FORBIDDEN_CREDENTIAL_USERINFO.search(text)) + + # Host-local infrastructure details must never enter the Git-authored # configuration through the free-form evidence locator (AGENTS.md). FORBIDDEN_EVIDENCE_ADDRESS = re.compile( r"(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}" - r"|\[[0-9a-f:]+\]" # bracketed IPv6 + r"|\[[0-9a-f:]+%?[^\]]*\]" # bracketed IPv6 with optional zone r"|[a-z0-9-]+(?:\.[a-z0-9-]+){2,}" # >=3-label hostname r"|[a-z0-9-]+(?:\.[a-z0-9-]+)*\.(?:internal|local|lan|corp|private|home|intranet)\b)", re.IGNORECASE, @@ -431,8 +460,13 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: continue host_group = environment.get("host_group") validation.require(host_group in groups, f"{path}.host_group", "must reference a declared deployment host group") - if host_group in groups and groups[host_group].get("role") != "deployment": - validation.require(False, f"{path}.host_group", "must reference a deployment-role host group; environments deploy only from deployment hosts") + referenced_group = groups.get(host_group) if isinstance(groups.get(host_group), dict) else None + if referenced_group is not None: + validation.require( + referenced_group.get("role") == "deployment", + f"{path}.host_group", + "must reference a deployment-role host group; environments deploy only from deployment hosts", + ) validation.require(type(environment.get("automatic")) is bool, f"{path}.automatic", "must be a boolean") requires_approval = environment.get("requires_approval") validation.require(type(requires_approval) is bool, f"{path}.requires_approval", "must be a boolean") @@ -450,8 +484,13 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: # fail-closed gate instead of rejecting every existing adopter # configuration (Codex finding, PR #14). mechanism = "github-environment" if environment_capable else "manual-external" + evidence_present = "approval_evidence" in environment evidence = environment.get("approval_evidence") - validation.require(evidence is None or (isinstance(evidence, str) and bool(evidence.strip())), f"{path}.approval_evidence", "must be a logical reference to where exact-head approval is recorded, never a secret value") + validation.require( + (not evidence_present and evidence is None) or (isinstance(evidence, str) and bool(evidence.strip())), + f"{path}.approval_evidence", + "must be a logical reference to where exact-head approval is recorded, never a secret value", + ) if mechanism == "github-environment" and not environment_capable: validation.require(False, f"{path}.approval_mechanism", "github-environment required-reviewer approval requires organization.github_plan enterprise; protected Environments and required reviewers are unavailable for private repositories on Free and Team — use manual-external") if mechanism == "manual-external" and requires_approval and not isinstance(evidence, str): @@ -468,12 +507,16 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if strict: validation.require(False, f"{path}.approval_evidence", "is an initializer placeholder; record a real approval locator (ticket, path, or system reference)") if isinstance(evidence, str): - # Complete ordered identity-phrase match against run-output - # vocabulary: punctuation cannot hide the CI identity - # ("trusted-ci/job-log", "trusted-ci: job log" both fail), and - # neither can prose between the identity and its output marker, - # but prose that merely reuses one generic word (the - # initializer's "release ticket") stays valid. + validation.require( + evidence_contains_credentials(evidence) is False, + f"{path}.approval_evidence", + "must not contain credential-bearing URI userinfo", + ) + validation.require( + not evidence_contains_bare_ipv6(evidence), + f"{path}.approval_evidence", + "must not contain host addresses or internal hostnames; reference the approval record by ticket, path, or system name only (AGENTS.md forbids infrastructure details in configuration)", + ) validation.require( FORBIDDEN_EVIDENCE_ADDRESS.search(evidence) is None, f"{path}.approval_evidence", @@ -517,7 +560,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(len(names) == len(set(names)), f"{path}.required_secret_names", "must contain unique names") for index, secret_name in enumerate(names): validation.require(isinstance(secret_name, str) and bool(SECRET_NAME.fullmatch(secret_name)), f"{path}.required_secret_names[{index}]", "must be an uppercase secret name, never a value") - if host_group in groups and groups[host_group].get("environment_class") == "production": + if host_group in groups and isinstance(groups.get(host_group), dict) and groups[host_group].get("environment_class") == "production": validation.require(environment.get("automatic") is False, f"{path}.automatic", "production deployment must not be automatic") validation.require(environment.get("requires_approval") is True, f"{path}.requires_approval", "production deployment must require approval") From 1298bdde9dd5405fb88e41cad208cafc57b6d01d Mon Sep 17 00:00:00 2001 From: Hermes Agent <1572453+Nickfost@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:42:26 -0500 Subject: [PATCH 09/14] fix(policy): reconcile round-7 Codex findings and vendor history-dependent legacy fixture - Require structured external approval locator (type:value) only for requires_approval environments so the initializer's development evidence and the committed CI init step stay valid (round-7: real external locator). - Reject token-only URI userinfo, pipeline/build self-approval markers, punctuated bare IPv6, and accept semantic versions in evidence locators. - Vendor the schema-v3 legacy fixture instead of reading e483998 from repository history, so templated adopter repos validate without the upstream ancestry (round-7: history-dependent legacy fixture). - Align the CI initializer step and README onboarding command with the structured locator requirement. Tests: 94 policy tests green; validate.sh, secret scan, syntax, and the initializer strict round-trip pass. --- .github/workflows/validate.yml | 2 +- README.md | 2 +- scripts/init.py | 9 +- scripts/test_policy.py | 153 ++++++++++++++++++++++++++++++++- scripts/validate.py | 26 ++++-- 5 files changed, 176 insertions(+), 16 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index dd92fd7..b7c5596 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -86,7 +86,7 @@ jobs: --organization test-company \ --project test-app \ --engine-ref "${engine_ref}" \ - --approval-evidence "release ticket RT-1042 in the ops tracker" \ + --approval-evidence "ticket:RT-1042 release approved in ops tracker" \ --output "${temporary_directory}/fleet.json" test "$(stat -c '%a' "${temporary_directory}/fleet.json")" = 644 # The placeholder evidence must fail strict until replaced: diff --git a/README.md b/README.md index 4b5c10c..fcd77d0 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ flowchart LR --capacity-budget 1 \ --max-runners 1 \ --engine-ref \ - --approval-evidence "" + --approval-evidence "ticket:RT-1042 where your production approval is recorded" ``` 3. Edit `fleet.json` to add the organization's real logical mappings. diff --git a/scripts/init.py b/scripts/init.py index c8e7c5c..3e7c364 100755 --- a/scripts/init.py +++ b/scripts/init.py @@ -71,12 +71,13 @@ def main() -> int: fail("--max-runners must not exceed --capacity-budget") if args.runner_memory_mib < 512: fail("--runner-memory-mib must be at least 512") + if args.github_plan != "enterprise" and not args.approval_evidence: + fail("--approval-evidence is required when --github-plan is not enterprise (structured locator: type:value, e.g. ticket:RT-1042 or url:https://tracker.example/RT-1042)") approval_mechanism = "github-environment" if args.github_plan == "enterprise" else "manual-external" - # A real locator must come from the operator: a generic prose sentence - # would name no actual approval record (Codex, PR #14 round 3). The - # placeholder is deliberately rejected by --strict until replaced. + # Structured locator required for manual-external: type:value (ticket, url, system, doc) + # The placeholder is deliberately rejected by --strict until replaced with a real locator. production_evidence = args.approval_evidence or ( - "REPLACE-ME: record where the exact reviewed commit SHA approval is kept" + "REPLACE-ME: structured locator for the exact reviewed commit SHA approval (type:value)" ) repository = args.repository or f"{args.organization}/{args.project}" diff --git a/scripts/test_policy.py b/scripts/test_policy.py index a18bb8c..b7f7a1e 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -51,6 +51,92 @@ def first_controller(config: dict) -> dict: return next(iter(config["controllers"].values())) +# Vendored schema-v3 fixture (pre-change e483998:fleet.json). Pinned as a +# string so the regression test cannot depend on upstream repository history; +# adopter repos created via "Use this template" have no e483998 ancestry +# (Codex finding, PR #14 round 7: vendor the legacy fixture instead of reading +# repository history). +LEGACY_V3_FLEET_JSON = """{ + "$schema": "./fleet.schema.json", + "schema_version": 3, + "organization": { + "slug": "example-org", + "registry": "ghcr.io/example-org", + "delivery_engine": "RandomDevelopment/ci-fleet", + "workflow_ref_policy": "immutable-commit" + }, + "runner_pools": { + "trusted-ci": { + "runner_group": "example-trusted-ci", + "routing_labels": ["docker-ci"], + "allowed_repositories": ["example-org/example-app"], + "public_repositories": false, + "capacity_budget": 1, + "job_submission_policy": "all-independent-jobs" + } + }, + "controllers": { + "example-ci-01": { + "pool": "trusted-ci", + "location": "example-site-a", + "state": "active", + "scale_set_name": "example-ci-01", + "lifecycle": "experimental", + "engine_ref": "8df97cc7575f47696fa82a179bbe39cd2874b1ca", + "min_runners": 0, + "max_runners": 1, + "runner_resources": { + "cpu_cores": 2, + "memory_mib": 4096 + } + } + }, + "host_groups": { + "development-apps": { + "role": "deployment", + "environment_class": "development" + }, + "production-apps": { + "role": "deployment", + "environment_class": "production" + } + }, + "environments": { + "development": { + "host_group": "development-apps", + "automatic": true, + "requires_approval": false, + "required_secret_names": ["DEPLOY_AUTH"] + }, + "production": { + "host_group": "production-apps", + "automatic": false, + "requires_approval": true, + "required_secret_names": ["DEPLOY_AUTH"] + } + }, + "projects": { + "example-app": { + "repository": "example-org/example-app", + "image": "ghcr.io/example-org/example-app", + "ci_pool": "trusted-ci", + "ci_contract": { + "runner_entrypoint": "./scripts/ci/run.sh", + "task_plan": "./scripts/ci/plan.json", + "aggregate_entrypoints": { + "fast": "./scripts/ci/run.sh fast", + "full": "./scripts/ci/run.sh full" + }, + "target_wall_clock_minutes": 5, + "max_job_minutes": 5, + "shard_target_minutes": 4 + }, + "deployments": ["development", "production"] + } + } +}""" + + class PolicyTests(unittest.TestCase): def assert_rejected(self, config: dict, expected: str, *, strict: bool = False) -> None: errors = errors_for(config, strict=strict) @@ -532,10 +618,10 @@ def test_pre_branch_v3_configuration_without_evidence_fields_stays_valid(self) - # fleet.json omitted both approval_mechanism AND approval_evidence. # Compatibility means that exact shape keeps validating; requiring # evidence for inferred manual-external would force adopters to edit - # data just to import the validator. - config = json.loads( - subprocess.check_output(["git", "show", "e483998:fleet.json"], text=True) - ) + # data just to import the validator. The fixture is vendored (not read + # from repository history) so freshly templated adopter repositories + # without the e483998 ancestry still pass (Codex, round 7). + config = json.loads(LEGACY_V3_FLEET_JSON) self.assertEqual(errors_for(config), []) def test_explicit_manual_gate_requires_evidence_in_every_mode(self) -> None: @@ -785,6 +871,65 @@ def test_credential_uri_userinfo_in_approval_evidence_is_rejected(self) -> None: config["environments"]["production"]["approval_evidence"] = evidence self.assert_rejected(config, "must not contain credential-bearing URI userinfo") + def test_token_only_uri_userinfo_in_approval_evidence_is_rejected(self) -> None: + # Codex, PR #14 round 7: userinfo without a colon (token@host) is a + # credential too; the colon-delimited regex must not be the only gate. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "approval at https://s3cr3t@example.com/RT-1042" + ) + self.assert_rejected(config, "must not contain credential-bearing URI userinfo") + + def test_ci_execution_pipeline_and_build_in_evidence_are_rejected(self) -> None: + # Codex, PR #14 round 7: pipeline/build are ordinary CI execution nouns. + config = copy.deepcopy(reference_config()) + for evidence in ( + "trusted-ci pipeline 123 approved exact reviewed commit SHA", + "trusted-ci build 123", + "trusted-ci pipelines 1 and 2 approved", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not name ordinary-CI state") + + def test_punctuated_bare_ipv6_in_approval_evidence_is_rejected(self) -> None: + # Codex, PR #14 round 7: punctuation around an unbracketed IPv6 literal + # must not let the address slip through whitespace-only tokenization. + config = copy.deepcopy(reference_config()) + for evidence in ( + "approval recorded on (2001:db8::1), ticket RT-1042", + "approved at [2001:db8::1]/RT-1042", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not contain host addresses or internal hostnames") + + def test_semantic_version_in_approval_evidence_is_accepted(self) -> None: + # Codex, PR #14 round 7: a dotted release version is not a hostname. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "release 1.2.3 approved in ticket RT-1042" + ) + self.assertEqual(errors_for(config), []) + + def test_production_without_structured_locator_is_rejected_strict(self) -> None: + # Codex, PR #14 round 7: an approval gate with no ticket/path/system + # locator must not pass strict validation. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "the exact reviewed commit SHA was approved" + ) + self.assert_rejected( + config, + "must be a structured external approval locator", + strict=True, + ) + + def test_legacy_v3_configuration_stays_valid_without_history(self) -> None: + # Codex, PR #14 round 7: vendored legacy fixture must validate when the + # upstream object (e483998) does not exist, i.e. in freshly templated + # adopter repositories. + config = json.loads(LEGACY_V3_FLEET_JSON) + self.assertEqual(errors_for(config), []) + if __name__ == "__main__": unittest.main() diff --git a/scripts/validate.py b/scripts/validate.py index ac3206c..2bfaf30 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -193,9 +193,9 @@ def evidence_tokens(text: str) -> list[str]: # meaning; replace with structured evidence fields (type + locator) if # false positives ever matter. EVIDENCE_RUN_OUTPUT_MARKERS = frozenset({ - "artifact", "artifacts", "action", "actions", "check", "checks", - "console", "job", "jobs", "log", "logs", - "output", "run", "runner", "runners", "runs", "stdout", + "artifact", "artifacts", "action", "actions", "build", "builds", + "check", "checks", "console", "job", "jobs", "log", "logs", + "output", "pipeline", "pipelines", "run", "runner", "runners", "runs", "stdout", "workflow", "workflows", }) @@ -213,13 +213,15 @@ def evidence_contains_bare_ipv6(text: str) -> bool: for token in text.split(): if token.startswith("[") and token.endswith("]"): continue - candidate, _ = _split_ipv6_zone(token) + # Strip surrounding punctuation before checking for bare IPv6 + stripped = token.strip(".,;:()[]{}<>\"'") + candidate, _ = _split_ipv6_zone(stripped) if candidate.count(":") >= 2 and re.fullmatch(r"[0-9a-f:]+", candidate, re.IGNORECASE): return True return False -FORBIDDEN_CREDENTIAL_USERINFO = re.compile(r"//[^@/\s]+:[^@/\s]+@") +FORBIDDEN_CREDENTIAL_USERINFO = re.compile(r"//[^@/\s]+(?:[:][^@/\s]+)?@") def evidence_contains_credentials(text: str) -> bool: @@ -228,10 +230,11 @@ def evidence_contains_credentials(text: str) -> bool: # Host-local infrastructure details must never enter the Git-authored # configuration through the free-form evidence locator (AGENTS.md). +# Semantic versions (major.minor.patch) are not hostnames. FORBIDDEN_EVIDENCE_ADDRESS = re.compile( r"(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}" r"|\[[0-9a-f:]+%?[^\]]*\]" # bracketed IPv6 with optional zone - r"|[a-z0-9-]+(?:\.[a-z0-9-]+){2,}" # >=3-label hostname + r"|(?!\d+(?:\.\d+){2}\b)[a-z0-9-]+(?:\.[a-z0-9-]+){2,}" # >=3-label hostname (not semantic version) r"|[a-z0-9-]+(?:\.[a-z0-9-]+)*\.(?:internal|local|lan|corp|private|home|intranet)\b)", re.IGNORECASE, ) @@ -507,6 +510,17 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: if strict: validation.require(False, f"{path}.approval_evidence", "is an initializer placeholder; record a real approval locator (ticket, path, or system reference)") if isinstance(evidence, str): + # Enforce structured external approval locator: type:value where type + # is one of ticket, url, system, doc (Codex, PR #14 round 6). Gate it + # on requires_approval so an initializer's non-gating evidence (the + # development environment) and legacy non-strict imports stay valid; + # a production gate with no real locator must still fail (round 7). + if strict and requires_approval: + validation.require( + ":" in evidence and evidence.count(":") >= 1 and evidence.index(":") > 0 and evidence.rindex(":") < len(evidence) - 1, + f"{path}.approval_evidence", + "must be a structured external approval locator (type:value, e.g. ticket:RT-1042, url:https://tracker.example/RT-1042, system:jira/PROJ-123, doc:releases/v1.2.3)", + ) validation.require( evidence_contains_credentials(evidence) is False, f"{path}.approval_evidence", From 693f9abd25007da986bffec7f88faf06c2075f29 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:20:49 -0500 Subject: [PATCH 10/14] fix(policy): close Codex round-8 evidence-locator bypass findings Validate the approval-evidence locator type (ticket:/url:/system:/doc:) instead of any colon, reject ordinary-CI execution URLs referenced directly as manual-external evidence, and reject unqualified single-label hosts. All three are bypasses of the existing structured-locator and address scan that Codex flagged at exact head 1298bdde; add regression tests for each. --- scripts/test_policy.py | 40 ++++++++++++++++++++++++++++++++++++++++ scripts/validate.py | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/scripts/test_policy.py b/scripts/test_policy.py index b7f7a1e..d299295 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -923,6 +923,46 @@ def test_production_without_structured_locator_is_rejected_strict(self) -> None: strict=True, ) + def test_prose_with_colon_is_not_a_locator_strict(self) -> None: + # Codex PR #14 round 8 finding 1: arbitrary prose containing a colon + # (e.g. an approval sentence) must not satisfy the structured-locator + # rule; only a typed prefix (ticket:/url:/system:/doc:) is a locator. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "the exact reviewed commit SHA was approved: yes" + ) + self.assert_rejected( + config, + "must be a structured external approval locator", + strict=True, + ) + + def test_ci_execution_url_in_evidence_is_rejected(self) -> None: + # Codex PR #14 round 8 finding 2: a manual-external locator that points + # straight at the requesting GitHub Actions run is still ordinary-CI + # self-approval and must be rejected regardless of configured identity. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "url:https://github.com/acme/app/actions/runs/123" + ) + self.assert_rejected( + config, + "must not reference ordinary-CI execution URLs", + ) + + def test_single_label_host_in_evidence_is_rejected(self) -> None: + # Codex PR #14 round 8 finding 3: an unqualified single-label host + # names a host-local service and leaks infrastructure details the + # evidence scan must block. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "url:http://ci-runner/RT-1042" + ) + self.assert_rejected( + config, + "must not contain unqualified single-label hosts", + ) + def test_legacy_v3_configuration_stays_valid_without_history(self) -> None: # Codex, PR #14 round 7: vendored legacy fixture must validate when the # upstream object (e483998) does not exist, i.e. in freshly templated diff --git a/scripts/validate.py b/scripts/validate.py index 2bfaf30..38e5520 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -224,6 +224,28 @@ def evidence_contains_bare_ipv6(text: str) -> bool: FORBIDDEN_CREDENTIAL_USERINFO = re.compile(r"//[^@/\s]+(?:[:][^@/\s]+)?@") +# Structured external-approval locators carry a typed prefix; a bare colon in +# arbitrary prose is not a locator (Codex PR #14 round 8). +EVIDENCE_LOCATOR_RE = re.compile(r"^(?:doc|system|ticket|url):[^\s]") + +# Ordinary-CI execution URLs (run/job/build/check logs) are self-approval +# evidence even when they omit the local pool/controller identity strings +# (Codex PR #14 round 8). +CI_EXECUTION_URL = re.compile( + r"https?://[^\s/]+(?:/[^\s/]+){2,}/(?:actions|runs?|jobs?|builds?|pipelines?|checks?)\b", + re.IGNORECASE, +) + + +def evidence_mentions_single_label_host(text: str) -> bool: + # Unqualified (single-label) hosts name host-local services and leak the + # infrastructure details the evidence scan must block (Codex PR #14 round 8). + for match in re.finditer(r"https?://([^\s/@]+)", text, re.IGNORECASE): + if "." not in match.group(1): + return True + return False + + def evidence_contains_credentials(text: str) -> bool: return bool(FORBIDDEN_CREDENTIAL_USERINFO.search(text)) @@ -517,7 +539,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: # a production gate with no real locator must still fail (round 7). if strict and requires_approval: validation.require( - ":" in evidence and evidence.count(":") >= 1 and evidence.index(":") > 0 and evidence.rindex(":") < len(evidence) - 1, + bool(EVIDENCE_LOCATOR_RE.search(evidence)), f"{path}.approval_evidence", "must be a structured external approval locator (type:value, e.g. ticket:RT-1042, url:https://tracker.example/RT-1042, system:jira/PROJ-123, doc:releases/v1.2.3)", ) @@ -526,6 +548,16 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: f"{path}.approval_evidence", "must not contain credential-bearing URI userinfo", ) + validation.require( + not evidence_mentions_single_label_host(evidence), + f"{path}.approval_evidence", + "must not contain unqualified single-label hosts; reference the approval record by ticket, path, or system name only (AGENTS.md forbids infrastructure details in configuration)", + ) + validation.require( + CI_EXECUTION_URL.search(evidence) is None, + f"{path}.approval_evidence", + "must not reference ordinary-CI execution URLs (actions/runs/jobs/builds/checks); approval evidence lives outside the requesting CI identity", + ) validation.require( not evidence_contains_bare_ipv6(evidence), f"{path}.approval_evidence", From dc06b7d944014506d33a1817d82169c32eaa0e48 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:35:42 -0500 Subject: [PATCH 11/14] fix(policy): close Codex round-8 evidence-locator and schema findings Enforce the structured approval locator for an explicitly declared manual-external gate in every mode (not only --strict), so the shipped template's own prose evidence can no longer pass the non-strict reference validation; record a real ticket: locator in fleet.json accordingly. Distinguish public multi-label SaaS approval URLs from host-local infrastructure: only IP literals, single-label hosts, and private suffixes are rejected, so url:https://acme.atlassian.net/... is accepted. Reject an explicit null organization.github_plan like the approval fields already reject explicit nulls. --- fleet.json | 2 +- scripts/test_policy.py | 70 +++++++++++++++++++++++++++++++++--------- scripts/validate.py | 39 +++++++++++++++++++---- 3 files changed, 90 insertions(+), 21 deletions(-) diff --git a/fleet.json b/fleet.json index 35ec414..97792aa 100644 --- a/fleet.json +++ b/fleet.json @@ -61,7 +61,7 @@ "automatic": false, "requires_approval": true, "approval_mechanism": "manual-external", - "approval_evidence": "signed release ticket recording the exact reviewed commit SHA before any privileged host accepts artifact inputs", + "approval_evidence": "ticket:RT-1042 signed release approval recording the exact reviewed commit SHA before any privileged host accepts artifact inputs", "required_secret_names": ["DEPLOY_AUTH"] } }, diff --git a/scripts/test_policy.py b/scripts/test_policy.py index d299295..b9cd76c 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -599,18 +599,18 @@ def test_evidence_identity_beside_punctuation_is_rejected(self) -> None: config["environments"]["production"]["approval_evidence"] = evidence self.assert_rejected(config, "must not name ordinary-CI state") - def test_prose_mention_of_a_generic_label_without_run_context_passes(self) -> None: - # Codex finding (PR #14): the initializer's own evidence text must - # stay valid when an operator picks a generic option value such as - # `--runner-label release`; prose reuse of one identity word is not - # self-approval, naming the CI run beside run-output context is. + def test_generic_label_prose_evidence_rejected_for_explicit_manual_gate(self) -> None: + # Codex finding (PR #14): prose reuse of one identity word is not + # self-approval, but an explicitly declared manual-external gate must + # still record a structured locator (round 8); generic prose fails + # closed every mode rather than silently passing CI. config = copy.deepcopy(reference_config()) config["runner_pools"]["trusted-ci"]["routing_labels"] = ["release"] config["environments"]["production"]["approval_evidence"] = ( "signed release ticket recording the exact reviewed commit SHA " "approved by the responsible engineer" ) - self.assertEqual(errors_for(config), []) + self.assert_rejected(config, "must be a structured external approval locator") self.assertEqual(errors_for(reference_config()), []) def test_pre_branch_v3_configuration_without_evidence_fields_stays_valid(self) -> None: @@ -649,20 +649,21 @@ def test_partial_identity_component_near_marker_is_accepted(self) -> None: # must not reject legitimate external evidence. config = copy.deepcopy(reference_config()) config["environments"]["production"]["approval_evidence"] = ( - "trusted release workflow ticket RT-1042" + "ticket:RT-1042 trusted release workflow approved" ) self.assertEqual(errors_for(config), []) - def test_initializer_placeholder_evidence_fails_strict(self) -> None: + def test_initializer_placeholder_evidence_fails_every_mode(self) -> None: # Codex finding, round 3 (PR #14): the initializer's generated - # production gate must not pass --strict with a generic prose - # sentence; the operator records a real locator via - # init.sh --approval-evidence. + # production gate must not pass with a generic prose/placeholder + # sentence; an explicit manual-external gate needs a real structured + # locator in every mode (round 8), and --strict additionally flags the + # REPLACE-ME placeholder wording. config = copy.deepcopy(reference_config()) config["environments"]["production"]["approval_evidence"] = ( "REPLACE-ME: record where the exact reviewed commit SHA approval is kept" ) - self.assertEqual(errors_for(config), []) + self.assert_rejected(config, "must be a structured external approval locator") self.assert_rejected(config, "initializer placeholder", strict=True) def test_controller_identity_in_evidence_is_rejected(self) -> None: @@ -804,7 +805,7 @@ def test_evidence_word_overlap_is_conservative_but_bounded(self) -> None: # no longer false-positive... config = copy.deepcopy(reference_config()) config["runner_pools"]["trusted-ci"]["runner_group"] = "trusted-ci" - config["environments"]["production"]["approval_evidence"] = "untrusted-city archive of signed approvals" + config["environments"]["production"]["approval_evidence"] = "doc:untrusted-city/archive/signed-approvals" self.assertEqual(errors_for(config), []) # ...but an exact reference to the CI identity still fails closed. config["environments"]["production"]["approval_evidence"] = "trusted-ci job log" @@ -906,7 +907,7 @@ def test_semantic_version_in_approval_evidence_is_accepted(self) -> None: # Codex, PR #14 round 7: a dotted release version is not a hostname. config = copy.deepcopy(reference_config()) config["environments"]["production"]["approval_evidence"] = ( - "release 1.2.3 approved in ticket RT-1042" + "ticket:RT-1042 release 1.2.3 approved" ) self.assertEqual(errors_for(config), []) @@ -963,6 +964,47 @@ def test_single_label_host_in_evidence_is_rejected(self) -> None: "must not contain unqualified single-label hosts", ) + def test_explicit_manual_gate_prose_fails_every_mode(self) -> None: + # Codex PR #14 round 8 finding 1: an explicitly declared manual-external + # gate must record a findable structured locator in every mode, not only + # strict; otherwise the shipped template's own prose evidence passes the + # non-strict Validate reference configurations CI job. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "signed release ticket recording the exact reviewed commit SHA" + ) + self.assert_rejected(config, "must be a structured external approval locator") + self.assert_rejected(config, "must be a structured external approval locator", strict=True) + + def test_public_saas_approval_url_is_accepted(self) -> None: + # Codex PR #14 round 8 finding 2: a public multi-label SaaS approval + # record such as Atlassian must satisfy the url: locator and not be + # mistaken for host-local infrastructure. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "url:https://acme.atlassian.net/browse/RT-1042" + ) + self.assertEqual(errors_for(config), []) + # The locator type is accepted in strict mode too; the example-org + # strict rejection is unrelated to the evidence locator. + config = copy.deepcopy(reference_config()) + config["organization"]["slug"] = "acme" + config["runner_pools"]["trusted-ci"]["allowed_repositories"] = ["acme/example-app"] + config["projects"]["example-app"]["repository"] = "acme/example-app" + config["projects"]["example-app"]["image"] = "ghcr.io/acme/example-app" + config["environments"]["production"]["approval_evidence"] = ( + "url:https://acme.atlassian.net/browse/RT-1042" + ) + self.assertEqual(errors_for(config, strict=True), []) + + def test_explicit_null_github_plan_is_rejected(self) -> None: + # Codex PR #14 round 8 finding 3: an explicit null is not key omission; + # the schema permits only free/team/enterprise, so the validator must + # reject null too (same explicit-null class as approval fields, round 5). + config = copy.deepcopy(reference_config()) + config["organization"]["github_plan"] = None + self.assert_rejected(config, "do not set it to null") + def test_legacy_v3_configuration_stays_valid_without_history(self) -> None: # Codex, PR #14 round 7: vendored legacy fixture must validate when the # upstream object (e483998) does not exist, i.e. in freshly templated diff --git a/scripts/validate.py b/scripts/validate.py index 38e5520..b06e4bd 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -246,6 +246,27 @@ def evidence_mentions_single_label_host(text: str) -> bool: return False +# Public multi-label SaaS approval services are legitimate external records; +# only IP literals, single-label names, and private suffixes are host-local +# infrastructure (Codex PR #14 round 8). Semantic versions (major.minor.patch) +# are not addresses. +EVIDENCE_PRIVATE_HOST = re.compile( + r"(?:localhost\b" + r"|\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}" + r"|\[[0-9a-f:]+%?[^\]]*\]" # bracketed IPv6 with optional zone + r"|[a-z0-9-]+(?:\.[a-z0-9-]+)*\.(?:internal|local|lan|corp|private|home|intranet)\b)", + re.IGNORECASE, +) + + +def evidence_host_is_private(text: str) -> bool: + # Reject an evidence locator that points at host-local infrastructure + # (IP literal, single-label host, or private suffix), so public + # multi-label approval URLs such as url:https://acme.atlassian.net/... + # stay accepted while address leaks are still blocked (Codex PR #14 round 8). + return bool(EVIDENCE_PRIVATE_HOST.search(text)) + + def evidence_contains_credentials(text: str) -> bool: return bool(FORBIDDEN_CREDENTIAL_USERINFO.search(text)) @@ -309,6 +330,11 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(engine == "RandomDevelopment/ci-fleet", "$.organization.delivery_engine", "must use the fixed reviewed public engine repository") validation.require(organization.get("workflow_ref_policy") == "immutable-commit", "$.organization.workflow_ref_policy", "must equal immutable-commit") plan = organization.get("github_plan") + validation.require( + "github_plan" not in organization or plan is not None, + "$.organization.github_plan", + "must be free, team, or enterprise; omit the key to mean free, do not set it to null", + ) validation.require(plan is None or (isinstance(plan, str) and plan in {"free", "team", "enterprise"}), "$.organization.github_plan", "must be free, team, or enterprise; omitted means free") if strict: validation.require(slug != "example-org", "$.organization.slug", "replace the example organization before use") @@ -533,11 +559,12 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(False, f"{path}.approval_evidence", "is an initializer placeholder; record a real approval locator (ticket, path, or system reference)") if isinstance(evidence, str): # Enforce structured external approval locator: type:value where type - # is one of ticket, url, system, doc (Codex, PR #14 round 6). Gate it - # on requires_approval so an initializer's non-gating evidence (the - # development environment) and legacy non-strict imports stay valid; - # a production gate with no real locator must still fail (round 7). - if strict and requires_approval: + # is one of ticket, url, system, doc (Codex, PR #14 round 6). An + # explicitly declared manual-external gate must record a findable + # locator in every mode; strict gating also covers inferred gates + # and legacy non-strict imports (round 7), so a production gate with + # no real locator cannot pass CI (Codex PR #14 round 8). + if (declared_mechanism == "manual-external" and requires_approval) or (strict and requires_approval): validation.require( bool(EVIDENCE_LOCATOR_RE.search(evidence)), f"{path}.approval_evidence", @@ -564,7 +591,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: "must not contain host addresses or internal hostnames; reference the approval record by ticket, path, or system name only (AGENTS.md forbids infrastructure details in configuration)", ) validation.require( - FORBIDDEN_EVIDENCE_ADDRESS.search(evidence) is None, + not evidence_host_is_private(evidence), f"{path}.approval_evidence", "must not contain host addresses or internal hostnames; reference the approval record by ticket, path, or system name only (AGENTS.md forbids infrastructure details in configuration)", ) From a28ead054e3fc058c79f16647e6eb823dcfb5389 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:49:20 -0500 Subject: [PATCH 12/14] fix(policy): close Codex round-9 credential, IPv6, and version findings Reject credential-bearing query/fragment parameters (token=/password=/sig=) in approval URLs, not only URI userinfo. Keep IPv6 compression colons during punctuation trimming so ::1 and fe80:: are still rejected. Require valid octets and digit delimiters for IPv4 so four-part calendar versions such as 2026.8.28.1 are not mistaken for addresses. Pass approval evidence in the initializer invalid-registry workflow case so it exercises registry validation instead of failing on the missing-evidence gate. --- .github/workflows/validate.yml | 1 + scripts/test_policy.py | 27 +++++++++++++++++++++++++ scripts/validate.py | 36 ++++++++++++++++++---------------- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index b7c5596..020b052 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -77,6 +77,7 @@ jobs: --project test-app \ --registry BAD \ --engine-ref "${engine_ref}" \ + --approval-evidence "ticket:RT-1042 registry review" \ --output "${temporary_directory}/invalid.json"; then echo 'initializer accepted an invalid registry' >&2 exit 1 diff --git a/scripts/test_policy.py b/scripts/test_policy.py index b9cd76c..27f94dd 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -1005,6 +1005,33 @@ def test_explicit_null_github_plan_is_rejected(self) -> None: config["organization"]["github_plan"] = None self.assert_rejected(config, "do not set it to null") + def test_credential_query_param_in_approval_url_is_rejected(self) -> None: + # Codex PR #14 round 9 finding 1: a query-authenticated or presigned + # approval URL leaks a secret through a parameter the userinfo check + # misses; reject credential-bearing query/fragment parameters. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "url:https://approvals.example.com/RT-1042?token=s3cr3t" + ) + self.assert_rejected(config, "must not contain credential-bearing URI userinfo") + + def test_compressed_ipv6_in_evidence_is_rejected(self) -> None: + # Codex PR #14 round 9 finding 2: leading/trailing compression colons + # must survive punctuation trimming so ::1 / fe80:: are still rejected. + config = copy.deepcopy(reference_config()) + for evidence in ("approval on ::1", "approval on fe80::"): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not contain host addresses or internal hostnames") + + def test_four_part_version_in_evidence_is_accepted(self) -> None: + # Codex PR #14 round 9 finding 3: a four-part calendar/release version + # is not IPv4; require valid octets so 2026.8.28.1 is not blocked. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "ticket:RT-1042 release 2026.8.28.1 approved" + ) + self.assertEqual(errors_for(config), []) + def test_legacy_v3_configuration_stays_valid_without_history(self) -> None: # Codex, PR #14 round 7: vendored legacy fixture must validate when the # upstream object (e483998) does not exist, i.e. in freshly templated diff --git a/scripts/validate.py b/scripts/validate.py index b06e4bd..3726918 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -213,8 +213,9 @@ def evidence_contains_bare_ipv6(text: str) -> bool: for token in text.split(): if token.startswith("[") and token.endswith("]"): continue - # Strip surrounding punctuation before checking for bare IPv6 - stripped = token.strip(".,;:()[]{}<>\"'") + # Strip surrounding punctuation but keep ':' so compressed IPv6 + # (::1, fe80::) is not mangled before parsing (Codex round 9). + stripped = token.strip(".,;()[]{}<>\"'") candidate, _ = _split_ipv6_zone(stripped) if candidate.count(":") >= 2 and re.fullmatch(r"[0-9a-f:]+", candidate, re.IGNORECASE): return True @@ -248,11 +249,12 @@ def evidence_mentions_single_label_host(text: str) -> bool: # Public multi-label SaaS approval services are legitimate external records; # only IP literals, single-label names, and private suffixes are host-local -# infrastructure (Codex PR #14 round 8). Semantic versions (major.minor.patch) -# are not addresses. +# infrastructure (Codex PR #14 round 8). A valid IPv4 requires octets <= 255, so +# four-part calendar/release versions (e.g. 2026.8.28.1) are not addresses +# (Codex round 9). Semantic versions (major.minor.patch) are not addresses. EVIDENCE_PRIVATE_HOST = re.compile( r"(?:localhost\b" - r"|\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}" + r"|(? bool: return bool(EVIDENCE_PRIVATE_HOST.search(text)) -def evidence_contains_credentials(text: str) -> bool: - return bool(FORBIDDEN_CREDENTIAL_USERINFO.search(text)) - - -# Host-local infrastructure details must never enter the Git-authored -# configuration through the free-form evidence locator (AGENTS.md). -# Semantic versions (major.minor.patch) are not hostnames. -FORBIDDEN_EVIDENCE_ADDRESS = re.compile( - r"(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}" - r"|\[[0-9a-f:]+%?[^\]]*\]" # bracketed IPv6 with optional zone - r"|(?!\d+(?:\.\d+){2}\b)[a-z0-9-]+(?:\.[a-z0-9-]+){2,}" # >=3-label hostname (not semantic version) - r"|[a-z0-9-]+(?:\.[a-z0-9-]+)*\.(?:internal|local|lan|corp|private|home|intranet)\b)", +# Credential-bearing query/fragment parameters (token/password/sig/key) leak +# secret values through approval URLs even without URI userinfo (Codex round 9). +FORBIDDEN_CREDENTIAL_PARAM = re.compile( + r"[?&#][^=&\s]*\b(?:token|password|passwd|secret|sig|signature|key|api[_-]?key)" + r"=[^\s&]+", re.IGNORECASE, ) +def evidence_contains_credentials(text: str) -> bool: + return bool( + FORBIDDEN_CREDENTIAL_USERINFO.search(text) + or FORBIDDEN_CREDENTIAL_PARAM.search(text) + ) + + def evidence_names_ci_state(evidence: str, identity: str) -> bool: """True when the evidence names the complete CI identity phrase. From 986616e284ef3465f25399510676a0b3df9cb1ba Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:55:40 -0500 Subject: [PATCH 13/14] fix(policy): close Codex round-10 compound credential parameter finding Cover compound OAuth/API credential names (access_token, client_secret, private_token) in approval URLs by allowing an optional _ or - separator before the keyword instead of a word boundary, which cannot occur after an underscore. --- scripts/test_policy.py | 13 +++++++++++++ scripts/validate.py | 5 ++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/scripts/test_policy.py b/scripts/test_policy.py index 27f94dd..7c9a881 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -1023,6 +1023,19 @@ def test_compressed_ipv6_in_evidence_is_rejected(self) -> None: config["environments"]["production"]["approval_evidence"] = evidence self.assert_rejected(config, "must not contain host addresses or internal hostnames") + def test_compound_credential_param_in_approval_url_is_rejected(self) -> None: + # Codex PR #14 round 10: compound OAuth/API param names such as + # access_token, client_secret, and private_token must be blocked even + # though a word boundary cannot occur after the connecting underscore. + config = copy.deepcopy(reference_config()) + for evidence in ( + "url:https://approvals.example.com/RT-1042?access_token=s3cr3t", + "url:https://approvals.example.com/RT-1042?client_secret=s3cr3t", + "url:https://git.example.com/RT-1042?private_token=s3cr3t", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not contain credential-bearing URI userinfo") + def test_four_part_version_in_evidence_is_accepted(self) -> None: # Codex PR #14 round 9 finding 3: a four-part calendar/release version # is not IPv4; require valid octets so 2026.8.28.1 is not blocked. diff --git a/scripts/validate.py b/scripts/validate.py index 3726918..9d8b992 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -271,8 +271,11 @@ def evidence_host_is_private(text: str) -> bool: # Credential-bearing query/fragment parameters (token/password/sig/key) leak # secret values through approval URLs even without URI userinfo (Codex round 9). +# Compound OAuth/API names such as access_token, client_secret, private_token +# are covered by allowing an optional _ or - separator before the keyword +# (Codex round 10). FORBIDDEN_CREDENTIAL_PARAM = re.compile( - r"[?&#][^=&\s]*\b(?:token|password|passwd|secret|sig|signature|key|api[_-]?key)" + r"[?&#][^=&\s]*(?:_|-)?(?:token|password|passwd|secret|sig|signature|key|api[_-]?key)" r"=[^\s&]+", re.IGNORECASE, ) From 422d3f975f7218fd9beda4cf2ce3dfa4bfa4c10c Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:03:43 -0500 Subject: [PATCH 14/14] fix(policy): close Codex round-11 credential-name, CI-marker, and scheme findings Cover the credential= query parameter name. Detect ordinary-CI execution segments (actions/runs/jobs/builds/pipelines/checks) at any path depth and under any scheme, not only after two http(s) path segments. Scan every url: scheme for single-label hosts, not only http(s), so ssh://ci-runner and similar internal hosts are still blocked. --- scripts/test_policy.py | 30 ++++++++++++++++++++++++++++++ scripts/validate.py | 11 +++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/scripts/test_policy.py b/scripts/test_policy.py index 7c9a881..590f1d6 100755 --- a/scripts/test_policy.py +++ b/scripts/test_policy.py @@ -1045,6 +1045,36 @@ def test_four_part_version_in_evidence_is_accepted(self) -> None: ) self.assertEqual(errors_for(config), []) + def test_credential_named_param_in_approval_url_is_rejected(self) -> None: + # Codex PR #14 round 11 finding 1: a credential= query parameter is an + # explicitly credential-bearing value the compound-name list omitted. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "url:https://approvals.example/RT?credential=s3cr3t" + ) + self.assert_rejected(config, "must not contain credential-bearing URI userinfo") + + def test_ci_execution_marker_at_any_depth_is_rejected(self) -> None: + # Codex PR #14 round 11 finding 2: a CI execution segment such as + # /pipelines must be rejected regardless of how many path segments + # precede it (e.g. CircleCI), and under any scheme. + config = copy.deepcopy(reference_config()) + for evidence in ( + "url:https://app.circleci.com/pipelines/github/acme/app/123", + "url:https://github.com/acme/app/actions/runs/123", + ): + config["environments"]["production"]["approval_evidence"] = evidence + self.assert_rejected(config, "must not reference ordinary-CI execution URLs") + + def test_non_http_scheme_single_label_host_is_rejected(self) -> None: + # Codex PR #14 round 11 finding 3: the url: locator accepts any scheme, + # so a single-label host under ssh:// must still be blocked. + config = copy.deepcopy(reference_config()) + config["environments"]["production"]["approval_evidence"] = ( + "url:ssh://ci-runner/RT-1042" + ) + self.assert_rejected(config, "must not contain unqualified single-label hosts") + def test_legacy_v3_configuration_stays_valid_without_history(self) -> None: # Codex, PR #14 round 7: vendored legacy fixture must validate when the # upstream object (e483998) does not exist, i.e. in freshly templated diff --git a/scripts/validate.py b/scripts/validate.py index 9d8b992..703033c 100755 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -231,9 +231,10 @@ def evidence_contains_bare_ipv6(text: str) -> bool: # Ordinary-CI execution URLs (run/job/build/check logs) are self-approval # evidence even when they omit the local pool/controller identity strings -# (Codex PR #14 round 8). +# (Codex PR #14 round 8). Detect known execution segments at any path depth +# and under any scheme (Codex round 11). CI_EXECUTION_URL = re.compile( - r"https?://[^\s/]+(?:/[^\s/]+){2,}/(?:actions|runs?|jobs?|builds?|pipelines?|checks?)\b", + r"[a-z][a-z0-9+.-]*://[^\s/]+(?:/[^/\s]+)*/(?:actions|runs?|jobs?|builds?|pipelines?|checks?)\b", re.IGNORECASE, ) @@ -241,7 +242,9 @@ def evidence_contains_bare_ipv6(text: str) -> bool: def evidence_mentions_single_label_host(text: str) -> bool: # Unqualified (single-label) hosts name host-local services and leak the # infrastructure details the evidence scan must block (Codex PR #14 round 8). - for match in re.finditer(r"https?://([^\s/@]+)", text, re.IGNORECASE): + # Scan any url: scheme, not only http(s), since the structured locator + # accepts arbitrary schemes (Codex round 11). + for match in re.finditer(r"[a-z][a-z0-9+.-]*://([^\s/@]+)", text, re.IGNORECASE): if "." not in match.group(1): return True return False @@ -275,7 +278,7 @@ def evidence_host_is_private(text: str) -> bool: # are covered by allowing an optional _ or - separator before the keyword # (Codex round 10). FORBIDDEN_CREDENTIAL_PARAM = re.compile( - r"[?&#][^=&\s]*(?:_|-)?(?:token|password|passwd|secret|sig|signature|key|api[_-]?key)" + r"[?&#][^=&\s]*(?:_|-)?(?:token|password|passwd|secret|sig|signature|key|api[_-]?key|credential)" r"=[^\s&]+", re.IGNORECASE, )