From b7258068005257f43edf863e9371637e4962b3a5 Mon Sep 17 00:00:00 2001 From: Rwanbt Date: Wed, 16 Sep 2026 22:35:18 +0200 Subject: [PATCH] feat(security): mandatory secret patterns as an unremovable floor, scenario closures (#166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the remaining traceable items from the Multi-Forge qualification report. - Candidate scanner (ainative/multivault/git_scanner.py): the constructor now takes `extra_secret_patterns` and no longer accepts a replacement set; the effective patterns are `MANDATORY_SECRET_PATTERNS + extras`, so no caller, repository or operator configuration can remove, replace, disable or shadow the floor (ADR-0019 section 12). The floor gains the documented GitLab token prefixes (`glpat-`, `gldt-`, `glrt-`, `glsoat-`), alongside private-key markers, AWS, GitHub and Slack patterns. The anti-debt owner (`finding_common.SECRET_PATTERNS`) and the vault-sync fallback carry the same GitLab prefixes. - Scenario J: the GitLab provider test proves an object-storage redirect on a package-file download is followed anonymously — the token stays at the API origin. - Scenario M: `doctor` warns when a GitLab remote is observed while the legacy `forge-github` default is only projected, naming the explicit `feature switch forge-gitlab` transition. - Scenario P: one end-to-end test asserts `update check`, `update`, `status` and `doctor` all surface the same `UPDATE_SOURCE_CONFLICT` for the same environment — one resolver, one refusal. - Documentation parity (plan section 75): `tests/purity/test_docs_parity.py` enforces the EN/FR README heading hierarchy, the operational surface (commands, env vars, config files) and the critical security statements — structurally, never by raw line counts. CI runs it with the purity suite. - Qualification report and CHANGELOG updated: scenarios J/M/P/Q and parity are GREEN; only the GitLab.com LIVE qualification and the release choreography remain, both declared. Verified: 314 lifecycle tests OK, 245 other suites OK (18 scanner, 110 release/claims/purity), gates green. Refs #166 --- .github/workflows/ci.yml | 4 +- CHANGELOG.md | 8 ++ ainative/multivault/git_scanner.py | 15 +++- ainative/observation.py | 12 ++- docs/MULTIFORGE-QUALIFICATION.md | 32 ++++---- scripts/vault_sync.py | 2 + .../agents/anti-debt/tools/finding_common.py | 4 + tests/purity/test_docs_parity.py | 78 +++++++++++++++++++ tests/test_forge_claims.py | 18 ++++- tests/test_multivault_git_scanner.py | 24 ++++++ tests/test_release_providers.py | 20 +++++ tests/test_release_source.py | 19 +++++ 12 files changed, 212 insertions(+), 24 deletions(-) create mode 100644 tests/purity/test_docs_parity.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31bebc4a..392fa0cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -219,9 +219,9 @@ jobs: - name: Release V3 update path end to end run: python -m unittest tests.test_lifecycle_update_v3 -v - - name: Purity - neutral layer, contracts, distributed policy + - name: Purity - neutral layer, contracts, distributed policy, docs parity run: | - python -m unittest tests.purity.test_dependency_purity tests.purity.test_contract_purity tests.purity.test_distributed_policy_purity -v + python -m unittest tests.purity.test_dependency_purity tests.purity.test_contract_purity tests.purity.test_distributed_policy_purity tests.purity.test_docs_parity -v - name: Path traversal, link escape, tampered state, archive safety run: python -m unittest tests.test_lifecycle_security -v diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe6c2fa..562c4caa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,14 @@ project adheres to [Semantic Versioning](https://semver.org/). canonical claim grammar and the durable `ainative claim-attempt list | inspect | abandon` journal — written before the remote signal, never retried blindly. +- **Mandatory secret patterns can only be extended** (#166, ADR-0019 §12): + the candidate scanner takes `extra_secret_patterns` (a union with + `MANDATORY_SECRET_PATTERNS`) and no longer offers any replacement + parameter; the mandatory floor now includes the documented GitLab token + prefixes (`glpat-`, `gldt-`, `glrt-`, `glsoat-`) in the scanner, the + anti-debt owner and the vault-sync fallback. EN/FR documentation parity is + enforced structurally (heading hierarchy, operational surface, security + statements). ### Security diff --git a/ainative/multivault/git_scanner.py b/ainative/multivault/git_scanner.py index 5bb2fd82..77dc69cb 100644 --- a/ainative/multivault/git_scanner.py +++ b/ainative/multivault/git_scanner.py @@ -20,7 +20,11 @@ from .schema import digest as canonical_digest -DEFAULT_SECRET_PATTERNS: tuple[bytes, ...] = ( +# The mandatory floor (ADR-0019 section 12): provider token prefixes and +# private-key markers every scanner in the stack keeps. Operators may ADD +# patterns (`extra_secret_patterns`), never remove, replace, disable or shadow +# these — which is why no constructor parameter accepts a replacement set. +MANDATORY_SECRET_PATTERNS: tuple[bytes, ...] = ( b"-----BEGIN RSA PRIVATE KEY", b"-----BEGIN OPENSSH PRIVATE KEY", b"-----BEGIN EC PRIVATE KEY", @@ -29,6 +33,10 @@ b"github_pat_", b"ghp_", b"xoxb-", + b"glpat-", + b"gldt-", + b"glrt-", + b"glsoat-", ) LARGE_BINARY_THRESHOLD_BYTES = 10 * 1024 * 1024 @@ -113,7 +121,7 @@ def __init__( repository: Path, *, forbidden_paths: tuple[str, ...] = (), - secret_patterns: tuple[bytes, ...] = DEFAULT_SECRET_PATTERNS, + extra_secret_patterns: tuple[bytes, ...] = (), max_binary_bytes: int = LARGE_BINARY_THRESHOLD_BYTES, git_executable: str | None = None, environment: Mapping[str, str] | None = None, @@ -127,7 +135,8 @@ def __init__( self._git = executable self._repository = Path(repository) self._forbidden_paths = tuple(forbidden_paths) - self._secret_patterns = tuple(secret_patterns) + # Extra patterns extend the mandatory floor; nothing replaces it. + self._secret_patterns = MANDATORY_SECRET_PATTERNS + tuple(extra_secret_patterns) self._max_binary_bytes = max_binary_bytes if environment is not None: validate_git_environment(environment) diff --git a/ainative/observation.py b/ainative/observation.py index 2878a5e5..d29d3fe8 100644 --- a/ainative/observation.py +++ b/ainative/observation.py @@ -86,7 +86,8 @@ def project_view(project: Path) -> dict: return { "features": {"active": list(effective.active_features) if effective else [], "projected_from_legacy": bool( - effective and effective.projected_from_legacy)}, + effective and effective.projected_from_legacy), + "legacy_default": featureslib.legacy_default(distribution)}, "forge": forge_picture(project), "claim_attempts": _claim_summary(project), } @@ -110,6 +111,15 @@ def doctor_lines(view: dict) -> list[str]: for remote in view["forge"]["remotes"]: lines.append(f" {remote['name']:<10} {remote['provider']:<8} " f"{remote['project'] or '-'}") + if features["projected_from_legacy"] and any( + remote["provider"] == "gitlab" for remote in view["forge"]["remotes"]): + # A legacy project with a GitLab remote keeps the compatibility default + # until it says otherwise; the switch is explicit, never inferred from + # the remote (ADR-0017 section 4 / scenario M). + lines.append(" note: a GitLab remote is observed while the legacy " + f"{features.get('legacy_default', 'forge-github')} default is " + "projected; run `ainative feature switch forge-gitlab` to " + "switch explicitly") if summary["journal"] == "ok": lines.append(f"Claim attempts: {len(summary['unresolved'])} unresolved") else: diff --git a/docs/MULTIFORGE-QUALIFICATION.md b/docs/MULTIFORGE-QUALIFICATION.md index 4b97e19a..3ee6e10a 100644 --- a/docs/MULTIFORGE-QUALIFICATION.md +++ b/docs/MULTIFORGE-QUALIFICATION.md @@ -51,14 +51,14 @@ Declared support never exceeds qualified support (`SUPPORT.md`, ADR-0019 §13). | G | Private GitHub asset → anonymous CDN | asset API flow test (scripted transport) | GREEN (scripted) | | H | Tampered ReleaseManifest V3 | `test_release_v3` + `test_lifecycle_update_v3` (zero writes) | GREEN | | I | Duplicate GitLab package/manifest | provider tests (`RELEASE_DUPLICATE_VERSION`, `RELEASE_MANIFEST_AMBIGUOUS`) | GREEN | -| J | GitLab object-storage blob | shared transport policy test (API → CDN strips credentials); no GitLab-specific 302 fixture yet | PARTIAL | +| J | GitLab object-storage blob | provider test: the manifest download 302s to an object store; the token is stripped on the blob hop and the bytes arrive | GREEN | | K | Claim crash after POST | journal tests (`UNCERTAIN` unresolved, no retry, explicit `abandon`) | GREEN | | L | Planner refusal during V1 projection | two-work-forge state refuses; projection never writes | GREEN | -| M | Legacy GitLab remote keeps GitHub compatibility default | projection test; the *warning* surfacing is not implemented | PARTIAL | +| M | Legacy GitLab remote keeps GitHub compatibility default | projection test + the doctor warning naming `feature switch forge-gitlab` | GREEN | | N | Read-only V1 projection == persisted V2 | migration parity test + `status` parity test | GREEN | | O | Missing legacy managed template not resurrected | `test_the_migration_does_not_resurrect_an_absent_template` | GREEN | -| P | Selector conflict identical across commands | one resolver consumed by update/check/status/doctor; conflicts tested at the resolver; no single cross-command E2E test | PARTIAL | -| Q | Mandatory secret patterns survive operator configuration | **not implemented** — the anti-debt scanner contract (`extra_secret_patterns`) is still pending | PENDING | +| P | Selector conflict identical across commands | one resolver; a single E2E asserting update check / update / status / doctor all surface `UPDATE_SOURCE_CONFLICT` | GREEN | +| Q | Mandatory secret patterns survive operator configuration | `MANDATORY_SECRET_PATTERNS` (private keys, AWS, GitHub, Slack, GitLab `glpat-`/`gldt-`/`glrt-`/`glsoat-`) with `extra_secret_patterns` as a union; the constructor accepts no replacement parameter; the anti-debt owner and the vault-sync fallback carry the same GitLab prefixes | GREEN | ## Security, migration, claims, integrity @@ -77,24 +77,22 @@ Declared support never exceeds qualified support (`SUPPORT.md`, ADR-0019 §13). ## Known limitations and remaining work -**P1 — blocks the declared-support gate:** +**P1 — the declared-support gate:** - GitLab.com live qualification (requires a real GitLab project publishing `ai-native-dev-stack` generic packages and an `ainative-release-v3.json` - manifest). -- Scenario Q: mandatory secret patterns (`MANDATORY_SECRET_PATTERNS` ∪ - operator extras) and the GitLab token-prefix qualification tuple. + manifest). Until then GitLab.com stays UNTESTED and is not declared + supported. - Full CI matrix (Linux/Windows/macOS, py3.11/3.13) — runs on the `dev` - proposal. -- EN/FR documentation parity automation (plan §75). - -**P2 — known gaps, documented rather than hidden:** -- Scenario J: no GitLab-specific object-storage 302 fixture (the shared - transport policy covers the behavior). -- Scenario M: the "legacy GitLab remote" case projects correctly but no - warning line is rendered yet. -- Scenario P: no single cross-command selector-conflict E2E test. + proposals; the first run of the complete implementation was green 52/52. - The V2 bridge release and the V3 release have not been published: the plan's release choreography (§92) starts after the branch is promoted to `dev`, and the first V3 publication stays gated by `V3_BRIDGE_RELEASE`. +**Resolved since the first draft of this report:** scenario Q (mandatory +secret patterns with operator extras), scenario J (GitLab object-storage +blob), scenario M (legacy GitLab remote warning), scenario P (one +cross-command selector-conflict E2E) and the EN/FR documentation parity +automation (`tests/purity/test_docs_parity.py`: heading hierarchy, operational +surface, critical security statements — no raw line-count equality). + **P0:** none known. diff --git a/scripts/vault_sync.py b/scripts/vault_sync.py index 8aaca2cc..db591ff8 100644 --- a/scripts/vault_sync.py +++ b/scripts/vault_sync.py @@ -105,6 +105,8 @@ def _load_secret_patterns() -> list: return [ (re.compile(r"AKIA[0-9A-Z]{16}"), "AWS access key ID"), (re.compile(r"gh[pousr]_[A-Za-z0-9]{36,}"), "GitHub token"), + (re.compile(r"glpat-[A-Za-z0-9_\-]{20,}"), "GitLab personal access token"), + (re.compile(r"gldt-[A-Za-z0-9_\-]{20,}"), "GitLab deploy token"), (re.compile(r"sk_live_[A-Za-z0-9]{20,}"), "Stripe live secret key"), (re.compile(r"-----BEGIN [A-Z ]+PRIVATE KEY-----"), "Private key"), ] diff --git a/stack/agents/anti-debt/tools/finding_common.py b/stack/agents/anti-debt/tools/finding_common.py index f462e9b2..fb607c27 100644 --- a/stack/agents/anti-debt/tools/finding_common.py +++ b/stack/agents/anti-debt/tools/finding_common.py @@ -31,6 +31,10 @@ (re.compile(r'AIza[0-9A-Za-z\-_]{35}'), "Google API key"), (re.compile(r'gh[pousr]_[A-Za-z0-9]{36,}'), "GitHub token"), (re.compile(r'xox[baprs]-[A-Za-z0-9-]{10,}'), "Slack token"), + (re.compile(r'glpat-[A-Za-z0-9_\-]{20,}'), "GitLab personal access token"), + (re.compile(r'gldt-[A-Za-z0-9_\-]{20,}'), "GitLab deploy token"), + (re.compile(r'glrt-[A-Za-z0-9_\-]{20,}'), "GitLab runner token"), + (re.compile(r'glsoat-[A-Za-z0-9_\-]{20,}'), "GitLab service account token"), (re.compile(r'-----BEGIN [A-Z ]+PRIVATE KEY-----'), "Private key"), (re.compile(r'(?i)(?:api[_-]?key|secret|token|passwd|password|access[_-]?key|client[_-]?secret)\s*[:=]\s*["\'][A-Za-z0-9+/_\-]{16,}["\']'), "generic credential"), ] diff --git a/tests/purity/test_docs_parity.py b/tests/purity/test_docs_parity.py new file mode 100644 index 00000000..d7abba1d --- /dev/null +++ b/tests/purity/test_docs_parity.py @@ -0,0 +1,78 @@ +"""EN/FR documentation parity, stated structurally (plan section 75). + +Raw line counts are not parity: a translation legitimately differs in length. +What must agree is structure — the heading hierarchy of the README pair — and +what must appear in both is the *operational surface* a user types: command +names, environment variables and configuration file names. A translated page +that silently drops a command or a security section fails here. +""" + +from __future__ import annotations + +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parents[2] +ENGLISH = REPO / "README.md" +FRENCH = REPO / "README.fr.md" + +FENCE = "```" + +# The operational surface introduced by Multi-Forge: present in both languages +# or the translation has fallen behind the tool. +OPERATIONAL_KEYS = ( + "ainative feature status", + "ainative feature switch forge-gitlab", + "ainative feature switch none", + "ainative forge status", + "ainative claim-attempt list", + "AINATIVE_UPDATE_URL", + "release-providers.json", + "FORGE-WORKFLOW.md", + "SUPPORT.md", +) + + +def heading_levels(text: str) -> list[int]: + """Heading levels outside fenced code blocks, in document order.""" + + levels: list[int] = [] + fence = False + for line in text.splitlines(): + stripped = line.strip() + if stripped.startswith(FENCE): + fence = not fence + continue + if fence: + continue + if stripped.startswith("#"): + levels.append(len(stripped) - len(stripped.lstrip("#"))) + return levels + + +class DocumentationParity(unittest.TestCase): + + def setUp(self) -> None: + self.english = ENGLISH.read_text(encoding="utf-8") + self.french = FRENCH.read_text(encoding="utf-8") + + def test_the_heading_hierarchy_is_identical_between_languages(self): + self.assertEqual(heading_levels(self.english), heading_levels(self.french), + "the EN/FR README structures drifted apart") + + def test_both_languages_speak_the_operational_surface(self): + for key in OPERATIONAL_KEYS: + with self.subTest(key=key): + self.assertIn(key, self.english) + self.assertIn(key, self.french) + + def test_the_critical_security_statements_appear_in_both(self): + # The statements, not the words: the French page says "ancré". + self.assertIn("fail-closed", self.english) + self.assertIn("anchored", self.english) + self.assertIn("fail-closed", self.french) + self.assertIn("ancré", self.french) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_forge_claims.py b/tests/test_forge_claims.py index 87c75afd..0e72dc44 100644 --- a/tests/test_forge_claims.py +++ b/tests/test_forge_claims.py @@ -335,13 +335,29 @@ def test_observation_commands_never_write(self): def test_doctor_reports_features_forge_and_claims(self): self.install("standard") - self.remote("origin", "git@gitlab.com:g/p.git") + self.remote("origin", "https://gitlab.com/g/p.git") record = json.loads(self.cli("doctor", "--json").stdout) self.assertEqual(record["features"]["active"], ["forge-github"]) self.assertEqual(record["forge"]["resolution"]["authority"]["provider"], "gitlab") self.assertEqual(record["claim_attempts"]["journal"], "ok") self.assertEqual(record["claim_attempts"]["unresolved"], []) + def test_a_legacy_gitlab_remote_keeps_the_default_with_a_warning(self): + """Scenario M: the choice stays explicit; the doctor says so.""" + + self.install("standard") + self.remote("origin", "https://gitlab.com/g/p.git") + path = statelib.state_path(self.project) + record = json.loads(path.read_text(encoding="utf-8")) + record["schema_version"] = 1 + record.pop("active_features", None) + statelib.write_atomic(path, json.dumps(record, indent=2, sort_keys=True) + "\n") + text = self.cli("doctor").stdout + self.assertIn("feature switch forge-gitlab", text) + reported = json.loads(self.cli("doctor", "--json").stdout) + self.assertEqual(reported["features"]["active"], ["forge-github"]) + self.assertTrue(reported["features"]["projected_from_legacy"]) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_multivault_git_scanner.py b/tests/test_multivault_git_scanner.py index 364437a0..33f3a4de 100644 --- a/tests/test_multivault_git_scanner.py +++ b/tests/test_multivault_git_scanner.py @@ -1,3 +1,4 @@ +import inspect import shutil import subprocess import tempfile @@ -71,6 +72,29 @@ def test_lfs_pointer_is_a_leak(self): self.assertEqual(ScanVerdict.LEAK, result.verdict) self.assertIn("lfs_pointer", [finding.category for finding in result.findings]) + def test_a_gitlab_token_is_a_leak(self): + source = commit_file(self.repo, "gitlab.txt", + b"token glpat-abcdefghijklmnopqrst\n") + result = self.scan(source) + self.assertEqual(ScanVerdict.LEAK, result.verdict) + self.assertIn("secret", [finding.category for finding in result.findings]) + + def test_extra_patterns_cannot_replace_the_mandatory_floor(self): + # The GitHub floor is still detected when an operator adds their own + # pattern: extras are a union, never a replacement (ADR-0019 §12). + source = commit_file(self.repo, "both.txt", + b"ghp_abcdefghijklmnop\nACME-INTERNAL-KEY-1234567890\n") + result = self.scan(source, extra_secret_patterns=(b"ACME-INTERNAL-KEY-",)) + self.assertEqual(ScanVerdict.LEAK, result.verdict) + details = " ".join(finding.detail for finding in result.findings) + self.assertIn("ghp_", details) + self.assertIn("ACME-INTERNAL-KEY-", details) + + def test_the_constructor_accepts_no_replacement_parameter(self): + parameters = inspect.signature(CandidateObjectScanner.__init__).parameters + self.assertIn("extra_secret_patterns", parameters) + self.assertNotIn("secret_patterns", parameters) + def test_missing_source_object_is_incomplete(self): unknown = "f" * 40 scanner = CandidateObjectScanner(self.repo) diff --git a/tests/test_release_providers.py b/tests/test_release_providers.py index 7851604b..27fae13b 100644 --- a/tests/test_release_providers.py +++ b/tests/test_release_providers.py @@ -472,6 +472,26 @@ def test_the_whole_chain_runs_against_a_scripted_gitlab(self): self.assertEqual(candidate.version, "2.5.0") self.assertEqual(payload, b"bundle-bytes") + def test_an_object_storage_redirect_strips_the_gitlab_token(self): + """Scenario J: the blob hop is anonymous, the API hop is not.""" + + manifest = anchored_manifest() + object_url = "https://objectstore.example/packages/77/blob" + server = self.server(manifest=manifest) + server.hops[MANIFEST_DOWNLOAD] = ("redirect", object_url) + server.hops[object_url] = manifest + provider = self.provider(server) + with mock.patch.object(transportlib, "_send", server.send), \ + mock.patch.dict("os.environ", {"GITLAB_TOKEN": "glpat-" + "x" * 20}): + candidate = provider.enumerate( + release_v3lib.ReleaseQuery("stable")).candidates[0] + payload = provider.fetch_manifest(candidate) + self.assertEqual(payload, manifest) + hops = {url: token for url, token, _accept in server.observed} + self.assertEqual(hops[MANIFEST_DOWNLOAD], "glpat-" + "x" * 20) + self.assertIsNone(hops[object_url], + "the object store received the GitLab token") + def test_fetching_before_enumerating_is_a_programming_refusal(self): provider = providerslib.GitLabReleaseProvider(GITLAB_ENDPOINT, "group/project") with self.assertRaises(LifecycleError) as raised: diff --git a/tests/test_release_source.py b/tests/test_release_source.py index a7f9f48c..e91ff8ca 100644 --- a/tests/test_release_source.py +++ b/tests/test_release_source.py @@ -220,6 +220,25 @@ def test_status_and_doctor_display_the_source_without_secrets(self): self.assertEqual(doctor["release_source"]["reason"], report["release_source"]["reason"]) + def test_a_selector_conflict_is_identical_across_commands(self): + """Scenario P: one resolver, one refusal, everywhere.""" + + self.install("standard") + environment = {"AINATIVE_UPDATE_LOCAL_DIR": str(self.root / "mirror")} + with mock.patch.dict(os.environ, environment): + check = self.cli("update", "check", "--strict", "--json") + self.assertEqual(check.returncode, 1) + checked = json.loads(check.stdout) + self.assertEqual(checked["status"], "CHECK_FAILED") + self.assertIn("never ordered", checked["detail"]) + update = self.cli("update", "--json") + self.assertEqual(update.returncode, 2) + self.assertEqual(json.loads(update.stdout)["error"], "UPDATE_SOURCE_CONFLICT") + status = json.loads(self.cli("status", "--json").stdout) + self.assertEqual(status["release_source"]["state"], "UPDATE_SOURCE_CONFLICT") + doctor = json.loads(self.cli("doctor", "--json").stdout) + self.assertEqual(doctor["release_source"]["state"], "UPDATE_SOURCE_CONFLICT") + if __name__ == "__main__": unittest.main()