feat(secrets,iac): fail-safe secret reachability gate (infra-admin P2 PR4/12)#843
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… recursion coverage (review) PR4 review fixes: - CRITICAL: Reachability now takes ctx and passes it into AccessChecker.CheckAccess so a slow/unreachable vault/aws probe is bounded by the route/pipeline deadline instead of hanging the pre-flight. step.iac_secret_reachability propagates its Execute ctx. - IMPORTANT: host-local backends (env/file/keychain) are now reachable ONLY for a local exec-env. For a remote exec-env they are fail-safe unreachable (ADR 0017: the remote agent resolves its own secrets; engine-host env/file/keychain are not verifiable on a remote runner). Closes a fail-open hole. - IMPORTANT: collectFromValue now also recurses typed []string (programmatically- built specs), with a dedicated TestCollectSecretRefs covering flat/map/[]any-of- strings/[]any-of-maps/[]string/double-nesting + dedup. - MINORS: assert CheckAccess called (VaultWithAccess) and called exactly once (MultipleRefs, proving provider-level verdict); add specs_from + specs/specs_from mutual-exclusion tests; DOCUMENTATION row notes provider-level verdict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a fail-safe “secret reachability” pre-flight capability intended to conservatively block IaC applies when the configured secrets backend cannot be verified from the chosen execution environment (especially for remote exec-envs per ADR 0017). This introduces a new secrets.Reachability API plus a step.iac_secret_reachability pipeline step and wires the step into the platform plugin’s step registry/docs.
Changes:
- Introduces
secrets.Reachability(ctx, provider, execEnv) Resultwith conservative host-local vs remote exec-env classification and AccessChecker probing. - Adds
step.iac_secret_reachabilityto collectsecret://references from IaC specs (static orspecs_from) and report per-ref reachability with a single provider-level probe. - Registers the new step type in the platform plugin and documents it in
DOCUMENTATION.md.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| secrets/reachability.go | New reachability classification/probing logic for secrets providers. |
| secrets/reachability_test.go | Unit tests for reachability behavior across provider categories and ctx propagation. |
| module/pipeline_step_iac_secret_reachability.go | New pipeline step to collect secret refs from specs and report reachability. |
| module/pipeline_step_iac_secret_reachability_test.go | External tests for step behavior (provider resolution, output shape, single probe). |
| module/pipeline_step_iac_secret_reachability_internal_test.go | Internal tests for secret-ref collection recursion/dedup/sorting. |
| plugins/platform/plugin.go | Registers step.iac_secret_reachability in StepTypes and StepFactories. |
| DOCUMENTATION.md | Documents the new step in the step catalog table. |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…il-safe (review/CI) - CI consistency gate: register step.iac_secret_reachability in coreModuleTypes (schema/schema.go) + ModuleSchema registry (module_schema.go) + StepSchema builtins (step_schema_builtins.go) + regenerate editor golden. (TestRegistryConsistency + TestCoreStepTypesHaveSchemas + TestModuleSchemaRegistry_* require this for every built-in step type.) - Copilot: Reachability rejects nil/typed-nil providers (fail-safe, was a potential panic/fail-open) via isNilProvider (reflect). - Copilot: step's specs_from resolving to empty/missing now ERRORS instead of silently returning all_reachable=true (was a fail-open gate bypass), matching iac_provider_plan/apply. - Copilot: corrected two inaccurate test comments. - Tests: TestReachability_NilProvider + TestSecretReachabilityStep_SpecsFromEmpty_Errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tory list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
intel352
added a commit
that referenced
this pull request
Jun 2, 2026
…v2.12.0) (#844) golangci-lint v2.12.0 (CI) govet flags 'Constant reflect.Ptr should be inlined' for the deprecated reflect.Ptr alias added in #843. Use the canonical reflect.Pointer. (Local golangci v2.11.4 did not flag this — version gap; main CI Lint went red on the #843 merge.) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR4/12 — fail-safe secret reachability gate (infra-admin Phase 2)
Locked plan Tasks 7–8; ADR 0017. A pre-flight that blocks a direct apply (→409) when a referenced secret can't be reached from the chosen exec-env. Additive — no consumer until the apply route wires it (scenario PR11).
Changes
secrets.Reachability(ctx, p Provider, execEnv) Result— fail-safef(backend × exec-env). Concrete-type classification: host-local (env/file/keychain) reachable ONLY for local exec-envs (""/local/local-docker); for a REMOTE exec-env they fail-safe to unreachable — the engine can't vouch for the remote agent's own secrets (ADR 0017 agent-side resolution; conservative until the agent-probe hardening lands).*GitHubSecretsProvider→ short-circuit unreachable (write-only) BEFORE any CheckAccess. Remotevault/aws→ reachable iffAccessChecker.CheckAccess(ctx)nil, else fail-safe; no-AccessChecker + remote → fail-safe unreachable. Never fails open.step.iac_secret_reachability— resolves the configured secrets provider, collects distinctsecret://refs from specs' config (recurses maps,[]any,[]string), returns{secrets:[{ref,reachable,reason}], all_reachable}. Accepts staticspecsorspecs_from(context). Provider-level verdict reported per ref (single CheckAccess).Review notes (resolved)
Reachabilitynow threads the caller'sctxintoCheckAccess(wascontext.Background(), which could hang the pre-flight ~60s ignoring the route deadline).[]string; added acollectSecretRefstable test (flat/map/slice/slice-of-maps/[]string/double-nested) + ctx-propagation + mutual-exclusion + specs_from tests.Verified:
go build ./...exit 0;secrets+module+plugins/all(incl. TestDocumentationCoverage) pass;golangci-lint --new-from-rev0 issues.🤖 Generated with Claude Code