diff --git a/TODO/TODO_credential_lifecycle.md b/TODO/TODO_credential_lifecycle.md index 53d1c58a145..72a30366f7c 100644 --- a/TODO/TODO_credential_lifecycle.md +++ b/TODO/TODO_credential_lifecycle.md @@ -1,98 +1,431 @@ -# Credential/Auth Convergence Tracker +# Credential Lifecycle Architecture (Reset) -Status date: 2026-02-09 -Owner: runtime/auth modeling +Status date: 2026-02-12 +Owner: runtime/auth modeling +Supersedes: checklist-style convergence tracker version -## Goal +## Why This Rewrite Exists -Make credentialed actions uniform across the codebase: +The prior document tracked migration tasks, but it mixed three different concerns: -1. Every credentialed interface declares required scopes. -2. Runtime derives credential acquisition from that contract. -3. Graph shape is consistent across local/dev/test/prod. -4. Missing contract is a hard failure, not a fallback path. +1. What is implemented now. +2. What abstraction boundaries we want. +3. What debt remains tactical vs architectural. -## Canonical Pattern +This rewrite resets around one rule: -For any credentialed workflow: +`credentialed workflows request capability intent, not provider mechanics` -`resolve_auth_contract -> cloud_env -> bind_secret -> cloud_credential -> execute(res:credential)` +## Problem Statement -Environment can change provider implementation, but the graph pattern must stay the same. +Current flows can work when environment assumptions happen to match local setup, but abstraction leakage makes auth brittle: -## Current Convergence State +- defaults and provider details leak into graph construction callsites, +- impersonation behavior is not selected through an explicit strategy layer, +- profile and secret source-of-truth are fragmented, +- lifecycle actions (acquire/create/rotate/verify) are not represented as one policy-driven pipeline. -- [x] Shared scope-contract primitives exist in `core/ir` (`ScopeContract`, `CredentialIntent`). -- [x] Gist interface declares a typed scope contract (`GistScopeContract`) and intent. -- [x] Gist graph uses cloud credential lifecycle chain. -- [x] Review graph declares typed scope contract (`ReviewScopeContract`) wrapping LLM contract. -- [x] LLM graph declares typed scope contract (`LlmScopeContract`) with provider-specific auth. -- [ ] Transport shell auth contract is standardized (explicit credential materialization semantics for shell flows). -- [x] CI guardrails prevent reintroduction of env-var credential nodes in tool graphs. +## Current State (Verified) -## Strict Invariants (Must Hold) +### Solid and worth keeping -- [x] No production tool graph may acquire credentials directly from env-var-only auth nodes. -- [x] Every credentialed action/request type exposes a scope contract. -- [x] Every `execute` node that requires auth must consume `res:credential`. -- [ ] Missing or invalid scope contract fails deterministically before transport I/O. +- Canonical chain shape is enforced by tests: + `resolve_auth -> cloud_env -> bind_secret -> cloud_credential -> execute(res:credential)` + (`gunbc-dag/tests/credential_chain.rs`) +- Typed contract primitives already exist: `ScopeContract`, `CredentialIntent` + (`core/ir/src/transport/scope.rs`) +- Credentialed interfaces are already contract-based (gist/review/llm transport modules). +- Discovery DAG exists for generating cloud config from live GCP state: + `lib/gcp-ops/src/discovery_graph.rs`, `lib/gcp-ops/src/discovery_ops.rs` -## Migration Checklist +### Half-implemented or mismatched abstractions -### A. Interface Contracts +- `CloudConfigResource::create()` does not yet execute discovery; it currently records manifest key/outputs only. + (`lib/cloud-ops/src/config_resource.rs`) +- Context/profile resolution now centralizes in `graph_cloud_config()` with deterministic source precedence, but compatibility fallback defaults still exist when no source is configured and strict mode is off. +- Profile/config precedence is specified for JSON/TOML/env sources, but repo-file loading and policy-file binding are still pending. -- [x] Add shared contract types in `core/ir/src/transport/scope.rs`. -- [x] Add gist scope contract in `core/ir/src/transport/gist.rs`. -- [x] Add review scope contract in `core/ir/src/transport/review.rs`. -- [x] Add LLM provider scope contract in `core/ir/src/transport/llm/mod.rs`. -- [x] Add compile-time tests for each contract type (unit tests in each module + `credential_chain.rs` regression tests). +## Target Architecture -### B. Graph Wiring +Credential lifecycle is modeled as five strict layers. -- [x] Migrate `lib/tools/gist/src/graph.rs` to cloud credential chain. -- [x] Migrate all auth-required tool graphs — all use canonical chain (gist, review, llm, github-credential). -- [x] Auth contract nodes emit `service/scheme/header_name/required_scopes` in all credentialed graphs. -- [x] Required scopes are visible in mock specs for every auth flow. -- [x] `build_cloud_credential_graph_for_runtime()` dispatches on both provider AND runtime (not just runtime). -- [x] Local dev auth refactored into `local_auth_upsert` sub-DAG with formal guard on create phase. +1. Intent layer (capability request) +- Input: `CredentialIntent` from interface contract. +- Output: normalized intent id + required scopes. +- Rule: callers say what they need, never how to fetch it. -### C. Tests and Mocks +2. Context layer (environment understanding) +- Input: runtime signals + profile selection. +- Output: `CredentialContext` (runtime kind, namespace, actor identity, candidate providers). +- Rule: env detection happens once here; downstream layers do not inspect env directly. -- [x] Update gist mock spec/integration mocks for cloud lifecycle nodes. -- [x] Regenerate testgen artifacts after auth graph migrations. -- [x] Add regression tests that assert no `credential_env` nodes in production tool graphs (`credential_chain.rs`). -- [x] Add tests that fail when contract scope list is empty (`credential_chain.rs`). -- [x] Update `gcp_local_mock_spec()` for `local_auth_upsert` sub-DAG boundaries. -- [x] Fix credential_lifecycle mock config to use `LocalDev` runtime for DryRun mode. -- [x] Fix `should_panic` expected message for `test_transport_mock_coverage_required`. +3. Policy layer (spec-driven decision) +- Input: intent + context. +- Output: `CredentialPolicy` (secret refs, scopes, impersonation rule, rotation policy, status). +- Rule: all intent-to-secret mapping lives in policy spec, not callsites. -### D. Enforcement +4. Provider strategy layer (concrete auth plan) +- Input: policy + context. +- Output: provider-specific execution plan (`gcp_wif_secret`, `adc_direct`, etc.). +- Rule: branching decisions such as impersonation happen here. -- [x] Add CI audit step: fail if auth-required graphs skip canonical credential chain (`credential_chain.rs` regression tests run in `cargo test --workspace`). -- [x] Add CI audit step: fail if new interface uses credentials without `ScopeContract` (scope contract validation tests in `credential_chain.rs`). -- [ ] Add runtime preflight error contract for missing scope declarations. +5. Execution layer (DAG apply) +- Input: provider plan. +- Output: credential lease + expiry + structured diagnostics. +- Rule: lifecycle actions (acquire/create/rotate/verify) run as explicit DAG steps. -### E. Cloud Env Layering +## Extraction from `the-gunbai` -- [x] Model cloud selectors layer (`CLOUD_PROVIDER`, `CLOUD_RUNTIME`) explicitly. -- [x] Model provider/runtime required vars via `CloudEnvRequirements`. -- [x] Model developer identity as required-any-of groups (for local GCP: `GCP_SECRETS_SA | GCP_SECRETS_IMPERSONATE_SA`). -- [x] Make `cloud_env` fail with aggregated missing requirements (not first-missing only). -- [x] Include first-class diagnosis path in errors (`make gist-auth-doctor RUNTIME=...`). -- [x] Add a provider-neutral `cloud-auth-doctor` tool (`lib/cloud-ops/src/auth_doctor.rs`); gist doctor delegates to it. -- [ ] Define/implement local developer profile source of truth and precedence (shell env vs managed profile). +The most reusable parts from `the-gunbai` are architecture patterns, not code copy: -## Acceptance Criteria +1. Credential flow algebra (typed auth strategy) +- `CredentialFlow` is an explicit enum of acquisition strategies: + `Stored`, `PlatformInjected`, `WorkloadIdentity`, `InteractiveAuth`, `Derived`, `Chained`. + (`../the-gunbai/crates/gunbai-types/src/credential.rs`) +- This cleanly separates "what flow class is this?" from provider details. -- [x] `make gist-recent` no longer depends on `GITHUB_TOKEN` env wiring in graph modeling. -- [x] Auth modeling for gist/review/llm follows one canonical chain shape. -- [x] Auth contract declarations are typed and colocated with action/request interfaces. -- [ ] No fallback-only auth paths remain undocumented. -- [x] CI has at least one hard guard preventing credential modeling drift. +2. Strict runtime/provisioning split +- Runtime secret reads are read-only; provisioning/upsert uses separate identity and command path. + (`../the-gunbai/docs/design/ci-secrets-minimal-invariants.md`) +- No implicit runtime upsert fallback. -## Remaining Open Items +3. Policy-driven auth config +- `secret-sources.toml` binds provider config + auth flow + per-secret mapping in one declarative model. + (`../the-gunbai/config/secret-sources.toml`) +- CI code resolves that model into concrete GCP OIDC -> STS -> optional impersonation execution. + (`../the-gunbai/crates/gunbai-ci/src/secrets.rs`) -- [ ] Transport shell auth contract standardization (explicit credential materialization semantics for shell flows). -- [ ] Runtime preflight error contract for missing scope declarations. -- [ ] Local developer profile source of truth and precedence (shell env vs managed profile). -- [ ] Document remaining fallback-only auth paths (if any exist). +4. First-class behavior patterns +- Authentication should be modeled as a reusable pattern (like `pattern/upsert`), not duplicated workflow glue. + (`../the-gunbai/docs/design/behavior-patterns.md`) + +5. Requirements as generic I/O contracts +- Secrets are treated as requirements within a generic prerequisite model (not a bespoke side subsystem). + (`../the-gunbai/docs/design/requirements-and-io.md`) + +6. Scope rigor by behavior +- Secret requirements merge scopes across understandings and support per-behavior scope lookup. + (`../the-gunbai/crates/gunbai-integrations-contracts/src/secret_validation.rs`) + +## Base `authenticate` Pattern (New Requirement for Gunbc) + +To make auth robust across the full codebase, define one foundational pattern: + +`pattern/authenticate` + +Every credentialed flow (gist/review/llm/cloud ops/future providers) must consume this pattern, not invent local auth logic. + +### Pattern contract + +Input: +- `CredentialIntent` (capability + scopes + scheme) +- `AuthContext` (runtime/profile/provider candidates) +- `AuthPolicy` (flow constraints and lifecycle policy) + +Output: +- `CredentialLease` (materialized credential + expiry + source metadata) +- `AuthDiagnostics` (structured path, phase failures, remediations) + +### Required phases + +1. `ResolveContext` +- detect runtime and profile using deterministic precedence. + +2. `SelectFlow` +- choose typed flow class: + `PlatformInjected | WorkloadIdentity | InteractiveAuth | Stored | Derived | Chained`. + +3. `AcquireBaseIdentity` +- gather initial token/credential seed for selected flow. + +4. `ExchangeOrDerive` +- run token exchange or derivation (STS, OAuth refresh, app token, etc.). + +5. `Impersonate` (conditional) +- executed only when strategy/policy requires it. + +6. `VerifyScopes` +- preflight required scopes/capabilities before downstream execute transport. + +7. `MaterializeLease` +- return uniform credential object with expiry and provenance. + +### Pattern invariants + +- No provider-specific branching in tool graphs. +- No implicit fallback auth path. +- Runtime auth read path is non-mutating. +- Provisioning/rotation paths are explicit, separate operations. +- Missing required scopes fail before external business transport I/O. + +## Spec Model (Gunb.ai Pattern, Gunbc Adaptation) + +Use two specs with a clean boundary: + +1. Discovered config (generated) +- Source: infra discovery DAG. +- Purpose: describe cloud topology/resources. +- Artifact: `CloudConfigSpec` (TOML/JSON, generated). + +2. Credential policy (authored) +- Source: repo-authored policy file(s). +- Purpose: describe auth behavior per intent and context. +- Fields include: + - required scopes, + - secret binding, + - provider preference, + - impersonation policy, + - rotation handler/max age, + - status (`active`/`deleted`) for reconcile/prune. + +This preserves gunbc’s DAG runtime while adopting spec-driven apply principles proven in `gunb.ai`. + +## Canonical Runtime Contract + +Graph shape stays canonical, but semantics tighten: + +`resolve_auth_contract -> resolve_context -> bind_policy -> cloud_credential(provider_plan) -> execute(res:credential)` + +Mapping from current node names: + +- `resolve_auth`: stays intent-focused. +- `cloud_env`: evolves into context/profile resolution. +- `bind_secret`: evolves into policy binding (not only naming composition). +- `cloud_credential`: executes strategy-selected provider plan. + +`cloud_credential` becomes the implementation of `pattern/authenticate` inside the canonical chain. + +## Reconciliation with Existing Gunbc DAGs + +This section maps the target architecture to the DAGs already in the repo so migration is incremental, not a rewrite. + +### Existing flow inventory (today) + +- Tool entry DAGs: + - `lib/tools/gist/src/graph.rs` (`build_gist_graph_with_config`) + - `lib/llm-ops/src/graph.rs` (`build_chat_completion_graph_with_config`) + - `lib/review/src/graph.rs` (`build_review_phase_graph_with_config`) + - `lib/cloud-ops/src/github_credential_graph.rs` (`build_github_credential_graph`) +- Provider-neutral cloud DAG: + - `lib/cloud-ops/src/graph.rs` (`build_cloud_secret_manager_credential_graph_from_config`) +- GCP credential DAG: + - `lib/gcp-ops/src/graph.rs` (`build_gcp_secret_manager_credential_graph`) +- GCP upsert DAG: + - `lib/gcp-ops/src/graph.rs` (`build_gcp_secret_manager_upsert_graph`) +- Local auth sub-DAG (shared): + - `lib/gcp-ops/src/graph.rs` (`build_local_auth_upsert_dag`) + +### Phase mapping (`pattern/authenticate` -> current nodes) + +1. `ResolveContext` +- Current implementation: + - Tool graphs use `cloud_env` via `CloudOps::ConstCloudConfig`. + - Cloud DAG normalizes config via `resolve_config` + `map_gcp_inputs`. +- Coverage: medium. +- Gap: precedence is centralized, but file-backed profile resolution and policy binding are not integrated yet. + +2. `SelectFlow` +- Current implementation: + - Provider/runtime dispatch in `build_cloud_secret_manager_credential_graph_from_config`. + - Runtime guard in `CloudOps::MapToGcpInputs`. +- Coverage: partial. +- Gap: selection is runtime/provider based, not policy + typed flow class. + +3. `AcquireBaseIdentity` +- Current implementation: + - GitHub runtime: `prepare_github_oidc -> execute -> parse_github_oidc`. + - Metadata runtime: `prepare_metadata_oidc -> execute -> parse_metadata_oidc`. + - Local runtime: `local_auth_upsert` sub-DAG (`check + ADC/OAuth refresh` path). +- Coverage: strong. + +4. `ExchangeOrDerive` +- Current implementation: + - `prepare_sts -> execute_sts -> parse_sts`. +- Coverage: strong. + +5. `Impersonate` (conditional) +- Current implementation: + - `prepare_impersonate -> execute_impersonate -> parse_impersonate`. +- Coverage: partial. +- Gap: currently unconditional in graph shape; `ShouldImpersonate` op exists but is not wired. + +6. `VerifyScopes` +- Current implementation: + - Contract typing and validation exists in `core/ir`. + - Gist/LLM/review/GitHub resolve auth nodes emit `required_scopes`. + - `required_scopes` now threads through `cloud_credential` inputs across credentialed tool flows. +- Dedicated `scope_preflight` nodes now gate business transport execute paths. +- Coverage: partial. +- Gaps: + - Scope preflight currently validates declaration presence/shape, not provider-granted effective scope sets. + +7. `MaterializeLease` +- Current implementation: + - `build_credential` constructs `Credential`. + - `expires_in` is surfaced by credential sub-DAG. +- Coverage: strong for materialization, partial for structured provenance diagnostics. + +### Reconciliation by workflow + +- Gist flow (`lib/tools/gist/src/graph.rs`): + - Best aligned today. + - Has `required_scopes`, `lifetime_seconds`, and `expires_in` wiring. + - Has explicit `scope_preflight` before gist execute. + - Gap: preflight validates declared scope IDs, not provider-granted effective scopes. + +- LLM flow (`lib/llm-ops/src/graph.rs` + `lib/llm-ops/src/lib.rs`): + - Canonical chain shape is correct. + - `LlmOps::ResolveAuth` emits `required_scopes` and passes them through `cloud_credential`. + - Has explicit `scope_preflight` before execute. + - Gap: preflight validates declared scope IDs, not provider-granted effective scopes. + +- Review flow (`lib/review/src/graph.rs`): + - Canonical chain shape is correct. + - Uses `ReviewOps::ResolveAuthContract` (backed by `ReviewScopeContract`) and carries review scopes. + - Has explicit `scope_preflight` before execute. + - Gap: preflight validates declared scope IDs, not provider-granted effective scopes. + +- GitHub credential validation flow (`lib/cloud-ops/src/github_credential_graph.rs`): + - Canonical chain shape is correct. + - `resolve_auth` now emits and threads `required_scopes`. + - Has explicit `scope_preflight` before execute. + - Gap: validation endpoint remains auth-presence focused only. + +- Cloud config resource path (`lib/cloud-ops/src/config_resource.rs`): + - Modeled as managed resource. + - Gap: `create()` currently does not invoke discovery DAG, so config lifecycle orchestration is incomplete. + +### Behavior-pattern alignment already present + +- Upsert pattern exists concretely for secret provisioning: + - `prepare_secret_get -> parse_secret_get -> prepare_secret_create -> prepare_secret_add_version`. +- Local auth sub-DAG now fails fast with explicit `gcloud auth application-default login` remediation when ADC is missing (no late impersonation 401 fallback path). +- Canonical tool-chain invariant is already enforced in tests: + - `gunbc-dag/tests/credential_chain.rs`. + +### Reconciliation decisions (authoritative) + +- Keep the external canonical chain node names in tool graphs. +- Implement `pattern/authenticate` inside `cloud_credential` internals first. +- Add `required_scopes` as required resolve_auth output in all credentialed flows. +- Add explicit `scope_preflight` node in credential chain before business `execute`. +- Wire conditional impersonation (`ShouldImpersonate`) in GCP credential and upsert DAGs. +- (Done) Replace direct `default_local_dev_config()` callsites with centralized context/profile resolver. + +## Keep / Refactor / Replace + +Keep: + +- existing `CredentialIntent` / `ScopeContract` types, +- canonical chain tests and guardrails, +- reusable GCP local auth and discovery ops. + +Refactor: + +- config/profile precedence and loading, +- `cloud_credential` internals to strategy + diagnostics, +- `bind_secret` semantics from “string binding” to policy binding. + +Replace/Add: + +- credential-policy spec + resolver, +- `pattern/authenticate` core module and tests, +- provider strategy interface, +- secret reconcile and rotation DAGs, +- explicit runtime preflight for missing/invalid required scopes. + +## Proposed Interfaces (Rust Sketch) + +```rust +trait CredentialResolver { + fn resolve(&self, req: CredentialRequest) -> Result; +} + +struct CredentialRequest { + intent: CredentialIntent, + interactive_allowed: bool, + ttl_seconds: Option, +} + +trait ContextResolver { + fn resolve(&self) -> Result; +} + +trait PolicyResolver { + fn resolve( + &self, + intent: &CredentialIntent, + context: &CredentialContext, + ) -> Result; +} + +trait ProviderStrategy { + fn plan( + &self, + policy: &CredentialPolicy, + context: &CredentialContext, + ) -> Result; +} +``` + +## Migration Plan + +Phase 0: Baseline diagnostics + +- Improve impersonation failures to include status and parsed error summary. +- Keep current graph shape unchanged while improving observability. + +Phase 1: Context/profile precedence + +- Implement deterministic precedence: + 1) explicit `--cloud-config` path, + 2) repo config (e.g. `.gunbc/config-.toml`), + 3) env overrides, + 4) fallback defaults (dev-only with explicit warning). +- Remove hidden hardcoded config from graph constructors. + +Phase 1.5: Introduce `pattern/authenticate` + +- Add a provider-neutral authenticate contract module in `core/ir`. +- Rewire existing `cloud_credential` internals to call pattern phases. +- Keep external graph shape unchanged while internals migrate. + +Phase 2: Policy binding + +- Introduce `credential-policy` schema and loader. +- Move intent->secret/scopes mapping from callsites into policy. +- Evolve `bind_secret` semantics to bind policy output. + +Phase 3: Strategy execution + +- Add provider strategy selection inside `cloud_credential`. +- Wire conditional impersonation. +- Support local flows that do not require impersonation when policy allows. + +Phase 4: Secret lifecycle apply loops + +- Add reconcile DAG: check/create/bind/upsert. +- Add rotation DAG: age check -> rotate handler -> version add -> verify. +- Add prune path for policy entries marked `deleted`. + +Phase 5: Hardening and cutover + +- Fail preflight before transport I/O if required scopes are absent/invalid. +- Add regression tests for precedence and policy resolution. +- Deprecate and then remove fallback-only behavior behind explicit compatibility gate. + +## Definition of Done + +- Credential behavior can be explained for every workflow as: + `intent -> context -> policy -> strategy -> execute` +- `make gist-recent` no longer depends on hidden hardcoded project/service-account defaults. +- Impersonation is conditional and diagnosable. +- Rotation is policy-driven, not per-workflow custom logic. +- Missing scope declarations fail before outbound network calls. +- Generated discovery config and authored credential policy are both first-class inputs. + +## Open Decisions + +- policy layout: single root file vs domain-split includes, +- namespace model: `local/dev/prod` flat vs inherited hierarchy, +- fallback compatibility window duration for existing env-driven setups. diff --git a/TODO/TODO_hacks.md b/TODO/TODO_hacks.md index c407c893c14..5cc1b7eda2b 100644 --- a/TODO/TODO_hacks.md +++ b/TODO/TODO_hacks.md @@ -138,3 +138,65 @@ Consolidated 4 copies of `build_cloud_credential_graph_for_runtime` into Removed `upsert_and_run` and unused imports (`CliToolError`, `Value`, `execute_cli_tool_op`, `HashMap`) from `lib/tools/clippy/src/ops.rs`. + +--- + +## 10. DAG typing is structural at edge-level, but node I/O is still dynamic + +**Where**: +- `core/ir/src/builder.rs` (strong edge/type/cardinality checks) +- `core/ir/src/types.rs` (`TypeId(pub String)`) +- `core/exec/src/lib.rs` (`Executable::execute(HashMap)`) +- `core/exec/src/execute.rs` (`execute_single_node` and input mock injection) +- `core/codegen/src/testgen/codegen.rs` (mock type checks skip `input_mocks`) + +**What happens**: +- We get strong DAG build-time guarantees for edge wiring: + - type compatibility + - cardinality compatibility + - fan-in rejection for scalar ports + - cycle prevention +- But node execution boundaries are still `HashMap`, so wrong + value types can still be injected at runtime (especially entrypoint ports). +- `MockSpec` type mismatch checks currently validate `transport_mocks` and + `boundary_mocks`, but not `input_mocks`. + +**Why it's a hack**: +- The model appears strongly typed end-to-end, but there is still a dynamic + escape hatch at node call boundaries. +- This is where regressions like optional bool/string coercion drift or + semantic placeholder values can bypass structural guarantees. + +**Supporting examples (current repo)**: +1. `parse_impersonate` expects a REST payload with `accessToken`, but the type + system only knows `TransportResponse`; shape-valid placeholders can still be + behavior-invalid (`lib/gcp-ops/src/ops.rs`). +2. `compare_*_content.check_mode` wrong-type tests (`Value::Str("")`) + are meaningful because node entrypoint inputs are runtime maps, not typed + structs (`gunbc-dag/src/*/generated_tests.rs`). +3. `BlobOps::CompareContent` relies on strict input extraction + (`optional_bool_strict`) to reject wrong-typed inputs at runtime + (`lib/blob/src/lib.rs`). +4. CLI parsing still has permissive coercion (`Int` uses parse-or-0) which is + runtime behavior, not structural typing (`core/cli/src/lib.rs`). + +**Suggested fix (incremental, DAG-first)**: +1. Add `input_mocks` type validation in testgen coverage checks (same level as + existing boundary/transport mock compatibility checks). +2. Generate typed node input/output wrappers from DAG signatures (e.g. + `ParseImpersonateIn`, `ParseImpersonateOut`) and use them in generated tests + + helper APIs. +3. Add typed entrypoint injection APIs (`set_input_typed`) to avoid ad-hoc + `Value` maps for common call paths. +4. Introduce refined semantic types for carrier payloads where needed: + - `ImpersonationResponse` (validated schema) + - `AccessToken` + - `ScopeSet` +5. Keep transport/world effects as runtime checks; push everything else to + DAG build-time or generated type wrappers. + +**Boundary of guarantee (explicit)**: +- Compile/build time can guarantee: DAG structure, port compatibility, + cardinality, typed wrappers, and mock shape/type correctness. +- Runtime must still validate: external provider payloads, auth scopes, + permissions, and freshness/availability of real resources. diff --git a/TODO/TODO_testgen_seed_policy_postmortem.md b/TODO/TODO_testgen_seed_policy_postmortem.md new file mode 100644 index 00000000000..2741e9e8928 --- /dev/null +++ b/TODO/TODO_testgen_seed_policy_postmortem.md @@ -0,0 +1,100 @@ +# Testgen Seed Policy Post-Mortem (Auth Regression) + +Status date: 2026-02-12 +Owner: codegen/testgen + auth modeling + +## Incident + +`make gist-recent` failed in real mode with: + +- `missing accessToken in impersonation response` + +Generated optional-input tests were seeding required semantic auth inputs with +shape-valid placeholders (for example shell `` response forms), which do +not satisfy parser semantics. + +## Root Cause + +The generator treated two different properties as equivalent: + +- structural/type validity ("this value has the right outer type") +- semantic validity ("this value is meaningful for this operation") + +For auth and transport carrier types, those are not equivalent. + +## Missing Pattern + +The missing abstraction is a **seed policy matrix** keyed by: + +- type class (what kind of value this is) +- test mode/context (how the node is being executed) + +Without that matrix, testgen defaults drift toward local heuristics and spot +fixes. + +## Rule We Added (Current Slice) + +For `Real` single-node optional-input tests: + +- required semantic-carrier inputs must be explicitly seeded from authored + data, not synthesized placeholders. +- accepted explicit seed sources: + - `MockSpec::input_mock` + - `MockSpec::node_example` + - `Node::with_example` +- if missing, generation hard-fails with a clear panic. + +Current semantic-carrier class includes: + +- `TransportRequest` +- `TransportResponse` +- `Credential` +- `Secret` +- `FilesystemHandle` +- `NetworkHandle` +- `ToolHandle` + +## General Pattern (Target, All Types/Modes) + +Policy must be centralized and deterministic: + +1. Classify type into seed class: + - `StructuralGeneratable` + - `SemanticCarrier` +2. Classify test context: + - `RealSingleNodeRequiredInput` + - `DryRunBoundaryMock` + - `LiveFlowInput` +3. Apply matrix: + - if class/context requires explicit seed: hard-fail on missing explicit seed + - otherwise allow witness/synthetic generation + +Seed provenance must be tracked and validated in priority order: + +- explicit (authored mock/example) +- witness (contract/type-derived) +- synthetic fallback (last resort) + +The key invariant: + +- semantic-carrier inputs are never silently satisfied by synthetic fallback in + contexts where behavior correctness is being asserted. + +## Why This Avoids Spot Fixes + +Failures become policy-driven, not node-driven: + +- new nodes automatically inherit rules by type class + mode +- missing authored seeds are caught at generation time +- no parser-local hacks (for example special-casing placeholder strings) + +## Follow-Up Work + +1. Move seed-class classification to a shared IR-level module so codegen/testgen + and future generators consume one source of truth. +2. Extend matrix enforcement beyond current slice: + - scenario generation contexts + - live-flow generation contexts +3. Add tests that assert unknown semantic carrier types fail closed unless + explicitly classified. +4. Keep parser behavior strict; no placeholder-specific parsing branches. diff --git a/TODO/consolidation.md b/TODO/consolidation.md index b0fbcfb2f88..d49779f11c8 100644 --- a/TODO/consolidation.md +++ b/TODO/consolidation.md @@ -645,6 +645,7 @@ compiler version is now captured directly, regardless of environment setup. - [x] Builder strings compile-time validation (`#[tool_target]`) — §6.1 - [x] Remove static CODEGEN_SOURCES path list — §6.3 - [ ] Design hermeticity annotation for `Shell` transport (see §8 design problem) +- [ ] Design DAG typing hardening plan (typed node I/O wrappers + input_mock type validation + semantic carrier refinements) — see `TODO/TODO_hacks.md` §10 ### Remaining (extension features — from architecture-debt.md §16) diff --git a/core/codegen/src/testgen/codegen.rs b/core/codegen/src/testgen/codegen.rs index a45b2c090bc..69fe5772488 100644 --- a/core/codegen/src/testgen/codegen.rs +++ b/core/codegen/src/testgen/codegen.rs @@ -36,6 +36,39 @@ use gunbc_test::{FermiCost, MockSpec, OutputMatcher, TestClass}; use serde_json::Value as JsonValue; use std::collections::{BTreeMap, HashMap, HashSet}; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SeedPolicy { + Generated, + ExplicitSeedRequired, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SeedContext { + RealSingleNodeRequiredInput, +} + +fn seed_policy_for_type(type_id: &str) -> SeedPolicy { + match type_id { + // Semantic carrier types: shape-valid placeholders are often behavior-invalid. + "TransportRequest" + | "TransportResponse" + | "Credential" + | "Secret" + | "FilesystemHandle" + | "NetworkHandle" + | "ToolHandle" => SeedPolicy::ExplicitSeedRequired, + _ => SeedPolicy::Generated, + } +} + +fn requires_explicit_seed(type_id: &str, context: SeedContext) -> bool { + match context { + SeedContext::RealSingleNodeRequiredInput => { + seed_policy_for_type(type_id) == SeedPolicy::ExplicitSeedRequired + } + } +} + /// Configuration for test generation. /// /// # What is NOT generated (proven by construction): @@ -1782,6 +1815,7 @@ impl<'a, T: Clone> TestGenerator<'a, T> { Result, Vec>, > = HashMap::new(); let mut skipped_nodes: HashSet = HashSet::new(); + let mut strict_seed_checked_nodes: HashSet = HashSet::new(); let lowered_ids = gunbc_exec::lower(self.dag) .unwrap_or_else(|err| { panic!( @@ -1850,6 +1884,10 @@ impl<'a, T: Clone> TestGenerator<'a, T> { } }; + if strict_seed_checked_nodes.insert(node_id.0.clone()) { + self.assert_optional_required_inputs_seeded(node, base_inputs); + } + // Missing optional input should not error. let mut inputs_missing = base_inputs.clone(); inputs_missing.remove(&port_name.0); @@ -1954,6 +1992,73 @@ impl<'a, T: Clone> TestGenerator<'a, T> { (tests, notes) } + fn has_explicit_node_input_seed(&self, node_id: &str, port_name: &str) -> bool { + if let Some(spec) = &self.mock_spec { + if spec + .input_mocks + .iter() + .any(|mock| mock.node == node_id && mock.port == port_name) + { + return true; + } + if spec + .node_examples + .iter() + .any(|example| example.node_id == node_id && example.inputs.contains_key(port_name)) + { + return true; + } + } + + self.dag + .get_node(&NodeId(node_id.to_string())) + .is_some_and(|node| { + node.examples + .iter() + .any(|example| example.inputs.contains_key(port_name)) + }) + } + + fn assert_optional_required_inputs_seeded( + &self, + node: &gunbc_ir::Node, + base_inputs: &BTreeMap, + ) { + let mut missing = Vec::new(); + + for port in &node.inputs { + let needs_value = port.has_guard() || !port.cardinality.allows_empty(); + if !needs_value { + continue; + } + if port.name.0 == "skip" && port.type_id.0 == "Bool" { + continue; + } + if !requires_explicit_seed( + port.type_id.0.as_str(), + SeedContext::RealSingleNodeRequiredInput, + ) { + continue; + } + if !base_inputs.contains_key(&port.name.0) + || !self.has_explicit_node_input_seed(&node.id.0, &port.name.0) + { + missing.push(format!("{}.{} ({})", node.id.0, port.name.0, port.type_id.0)); + } + } + + if missing.is_empty() { + return; + } + + panic!( + "Optional input tests require explicit seeds for required semantic inputs in Real single-node mode.\n\ + Missing explicit seeds: {}.\n\ + Add one of: MockSpec::input_mock(\"\", \"\", ...), MockSpec::node_example(...), or Node::with_example(...).", + missing.join(", ") + ); + } + // ======================================================================= // Bucket C: Scenario Coverage // ======================================================================= @@ -2950,6 +3055,22 @@ impl<'a, T: Clone> TestGenerator<'a, T> { } } } + + // Seed known per-node inputs from MockSpec before generic synthesis. + if let Some(spec) = &self.mock_spec { + for input_mock in &spec.input_mocks { + if input_mock.node != node_id.0 { + continue; + } + if !node.inputs.iter().any(|port| port.name.0 == input_mock.port) { + continue; + } + inputs + .entry(input_mock.port.clone()) + .or_insert_with(|| ValueExpr::from(&input_mock.value)); + } + } + let mut issues = Vec::new(); for port in &node.inputs { @@ -5594,6 +5715,108 @@ mod tests { ); } + #[test] + fn test_optional_inputs_use_mockspec_input_mocks_for_required_ports() { + use gunbc_ir::transport::{RestResponse, TransportResponse}; + + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque( + "parse", + vec![ + port("response", "TransportResponse"), + optional("fallback", "OptionalString"), + ], + vec![port("result", "String")], + (), + )); + + let spec = MockSpec::new("opt") + .input_mock( + "parse", + "response", + Value::Response(TransportResponse::Rest(RestResponse::ok( + serde_json::json!({ "result": "ok" }), + ))), + ) + .boundary("parse", "result", Value::Str("ok".into())) + .skip_node_example("parse"); + + let generator = TestGenerator::new(&dag) + .with_mock_spec(spec) + .with_mock_spec_fn("crate::mock_spec()"); + let code = generator.generate_test_module("opt", "build_opt_graph()"); + + assert!( + code.contains("test_optional_missing_parse_fallback"), + "should generate missing-optional test" + ); + assert!( + code.contains("TransportResponse::Rest"), + "required inputs should come from MockSpec input_mock values" + ); + assert!( + !code.contains( + "TransportResponse::Shell(gunbc_ir::transport::ShellResponse { exit_code: 0, stdout: \"\"" + ), + "should not synthesize generic shell placeholders when input mocks exist" + ); + } + + #[test] + fn test_seed_policy_marks_semantic_types_explicit() { + assert_eq!( + seed_policy_for_type("TransportResponse"), + SeedPolicy::ExplicitSeedRequired + ); + assert_eq!( + seed_policy_for_type("TransportRequest"), + SeedPolicy::ExplicitSeedRequired + ); + assert_eq!( + seed_policy_for_type("Credential"), + SeedPolicy::ExplicitSeedRequired + ); + assert_eq!( + seed_policy_for_type("Secret"), + SeedPolicy::ExplicitSeedRequired + ); + assert_eq!(seed_policy_for_type("String"), SeedPolicy::Generated); + assert_eq!(seed_policy_for_type("Int"), SeedPolicy::Generated); + + assert!(requires_explicit_seed( + "TransportResponse", + SeedContext::RealSingleNodeRequiredInput + )); + assert!(!requires_explicit_seed( + "String", + SeedContext::RealSingleNodeRequiredInput + )); + } + + #[test] + #[should_panic(expected = "Optional input tests require explicit seeds for required semantic inputs in Real single-node mode")] + fn test_optional_inputs_require_explicit_semantic_seed() { + let mut dag: Dag<()> = Dag::new(); + dag.add_node(Node::opaque( + "parse", + vec![ + port("response", "TransportResponse"), + optional("fallback", "OptionalString"), + ], + vec![port("result", "String")], + (), + )); + + let spec = MockSpec::new("opt") + .boundary("parse", "result", Value::Str("ok".into())) + .skip_node_example("parse"); + + let generator = TestGenerator::new(&dag) + .with_mock_spec(spec) + .with_mock_spec_fn("crate::mock_spec()"); + let _ = generator.generate_test_module("opt", "build_opt_graph()"); + } + #[test] fn test_generate_with_satisfies_matcher_uses_runtime_check() { use gunbc_test::{NodeExample, OutputMatcher}; diff --git a/core/ir/src/transport/credential_policy.rs b/core/ir/src/transport/credential_policy.rs new file mode 100644 index 00000000000..3f77af5362c --- /dev/null +++ b/core/ir/src/transport/credential_policy.rs @@ -0,0 +1,566 @@ +//! Credential policy schema (v0) and profile inheritance resolution. +//! +//! This models policy-driven auth selection without tying callers to provider +//! mechanics. The schema is intentionally provider-neutral and supports +//! inherited profiles (`base -> dev/prod`) with per-intent overrides. + +use super::cloud::{CloudProviderKind, CloudRuntimeKind}; +use serde::{Deserialize, Serialize}; +use std::collections::{BTreeMap, BTreeSet}; + +/// Top-level credential policy document. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CredentialPolicySpec { + /// Schema version (starts at 0 during rollout). + #[serde(default)] + pub version: u32, + /// Optional default profile when the caller does not specify one. + #[serde(skip_serializing_if = "Option::is_none")] + pub default_profile: Option, + /// Named profiles (e.g., base/dev/prod/local). + #[serde(default)] + pub profiles: Vec, +} + +impl CredentialPolicySpec { + /// Look up a profile by name. + pub fn profile(&self, name: &str) -> Option<&CredentialPolicyProfile> { + self.profiles.iter().find(|p| p.name == name) + } + + /// Validate profile and intent uniqueness. + pub fn validate(&self) -> Result<(), CredentialPolicyError> { + let mut seen_profiles = BTreeSet::new(); + for profile in &self.profiles { + if !seen_profiles.insert(profile.name.clone()) { + return Err(CredentialPolicyError::DuplicateProfile(profile.name.clone())); + } + + let mut seen_intents = BTreeSet::new(); + for intent in &profile.intents { + if !seen_intents.insert(intent.intent.clone()) { + return Err(CredentialPolicyError::DuplicateIntent { + profile: profile.name.clone(), + intent: intent.intent.clone(), + }); + } + } + } + Ok(()) + } + + /// Resolve a profile with inherited defaults and intent policies. + pub fn resolve_profile( + &self, + name: &str, + ) -> Result { + self.validate()?; + self.resolve_profile_inner(name, &mut Vec::new()) + } + + /// Resolve one intent policy for a given profile. + pub fn resolve_intent_policy( + &self, + profile_name: &str, + intent_key: &str, + ) -> Result { + let resolved = self.resolve_profile(profile_name)?; + let mut policy = resolved + .intents + .get(intent_key) + .cloned() + .ok_or_else(|| CredentialPolicyError::UnknownIntent { + profile: profile_name.to_string(), + intent: intent_key.to_string(), + })?; + + if policy.provider.is_none() { + policy.provider = resolved.defaults.provider; + } + if policy.runtime.is_none() { + policy.runtime = resolved.defaults.runtime; + } + if policy.version_selector.is_none() { + policy.version_selector = resolved.defaults.version_selector.clone(); + } + if policy.impersonation.is_none() { + policy.impersonation = resolved.defaults.impersonation.clone(); + } + + if policy.secret.is_none() { + return Err(CredentialPolicyError::MissingSecretBinding { + profile: profile_name.to_string(), + intent: intent_key.to_string(), + }); + } + + if policy.version_selector.is_none() { + policy.version_selector = Some(VersionSelector::Latest); + } + + if policy.impersonation.is_none() { + policy.impersonation = Some(ImpersonationPolicy::default()); + } + + Ok(policy) + } + + fn resolve_profile_inner( + &self, + name: &str, + stack: &mut Vec, + ) -> Result { + if stack.iter().any(|n| n == name) { + let mut cycle = stack.clone(); + cycle.push(name.to_string()); + return Err(CredentialPolicyError::InheritanceCycle(cycle)); + } + + let profile = + self.profile(name) + .ok_or_else(|| CredentialPolicyError::UnknownProfile(name.to_string()))?; + + stack.push(name.to_string()); + + let mut resolved = if let Some(parent) = &profile.inherits_from { + self.resolve_profile_inner(parent, stack)? + } else { + ResolvedCredentialPolicyProfile::new(profile.name.clone()) + }; + + resolved.name = profile.name.clone(); + resolved.defaults = resolved.defaults.merged_with(&profile.defaults); + + for intent in &profile.intents { + let merged = match resolved.intents.get(&intent.intent) { + Some(parent_intent) => { + merge_intent_policy(parent_intent, intent, &resolved.defaults.scope_merge) + } + None => merge_intent_policy( + &ResolvedCredentialIntentPolicy::new(intent.intent.clone()), + intent, + &resolved.defaults.scope_merge, + ), + }; + resolved.intents.insert(intent.intent.clone(), merged); + } + + let _ = stack.pop(); + Ok(resolved) + } +} + +/// One policy profile. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CredentialPolicyProfile { + /// Profile name (e.g., base, local, dev, prod). + pub name: String, + /// Optional parent profile name. + #[serde(skip_serializing_if = "Option::is_none")] + pub inherits_from: Option, + /// Profile-wide defaults. + #[serde(default)] + pub defaults: CredentialPolicyDefaults, + /// Per-intent policy entries. + #[serde(default)] + pub intents: Vec, +} + +/// Profile-wide defaults applied to all intents unless overridden. +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)] +pub struct CredentialPolicyDefaults { + #[serde(skip_serializing_if = "Option::is_none")] + pub provider: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub runtime: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub impersonation: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub version_selector: Option, + #[serde(default)] + pub scope_merge: ScopeMergeMode, +} + +impl CredentialPolicyDefaults { + fn merged_with(&self, child: &CredentialPolicyDefaults) -> CredentialPolicyDefaults { + CredentialPolicyDefaults { + provider: child.provider.or(self.provider), + runtime: child.runtime.or(self.runtime), + impersonation: child.impersonation.clone().or_else(|| self.impersonation.clone()), + version_selector: child + .version_selector + .clone() + .or_else(|| self.version_selector.clone()), + scope_merge: if child.scope_merge == ScopeMergeMode::Replace { + self.scope_merge + } else { + child.scope_merge + }, + } + } +} + +/// Intent-level policy entry. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CredentialIntentPolicy { + /// Stable intent key (example: "github.gist.create"). + pub intent: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub provider: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub runtime: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub secret: Option, + /// `None` means inherit parent/default scopes. + #[serde(skip_serializing_if = "Option::is_none")] + pub required_scopes: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub scope_merge: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub impersonation: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub version_selector: Option, +} + +/// How scope lists merge during profile inheritance. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +#[derive(Default)] +pub enum ScopeMergeMode { + /// Child replaces parent scopes when `required_scopes` is present. + #[default] + Replace, + /// Child scopes are unioned with parent scopes. + Union, +} + + +/// Secret binding reference used by an intent policy. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SecretBinding { + /// Logical secret name/id (without profile prefixing assumptions). + pub name: String, + /// Optional explicit prefix override. + #[serde(skip_serializing_if = "Option::is_none")] + pub prefix: Option, + /// Optional delimiter override. + #[serde(skip_serializing_if = "Option::is_none")] + pub delimiter: Option, +} + +/// Policy-driven impersonation behavior. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "mode", rename_all = "snake_case")] +pub enum ImpersonationPolicy { + Never, + IfConfigured { + #[serde(skip_serializing_if = "Option::is_none")] + service_account: Option, + }, + Always { + service_account: String, + }, +} + +impl Default for ImpersonationPolicy { + fn default() -> Self { + Self::IfConfigured { + service_account: None, + } + } +} + +/// Secret version selector policy. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum VersionSelector { + Latest, + Alias { name: String }, + Fixed { version: String }, +} + +/// Resolved profile with merged defaults/intents. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedCredentialPolicyProfile { + pub name: String, + pub defaults: CredentialPolicyDefaults, + pub intents: BTreeMap, +} + +impl ResolvedCredentialPolicyProfile { + fn new(name: String) -> Self { + Self { + name, + defaults: CredentialPolicyDefaults::default(), + intents: BTreeMap::new(), + } + } +} + +/// Resolved intent policy after inheritance merge. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedCredentialIntentPolicy { + pub intent: String, + pub provider: Option, + pub runtime: Option, + pub secret: Option, + pub required_scopes: Vec, + pub impersonation: Option, + pub version_selector: Option, +} + +impl ResolvedCredentialIntentPolicy { + fn new(intent: String) -> Self { + Self { + intent, + provider: None, + runtime: None, + secret: None, + required_scopes: Vec::new(), + impersonation: None, + version_selector: None, + } + } +} + +fn merge_intent_policy( + parent: &ResolvedCredentialIntentPolicy, + child: &CredentialIntentPolicy, + default_scope_mode: &ScopeMergeMode, +) -> ResolvedCredentialIntentPolicy { + let scope_mode = child.scope_merge.unwrap_or(*default_scope_mode); + let required_scopes = match (&child.required_scopes, scope_mode) { + (Some(scopes), ScopeMergeMode::Replace) => scopes.clone(), + (Some(scopes), ScopeMergeMode::Union) => { + let mut set = BTreeSet::new(); + for s in &parent.required_scopes { + set.insert(s.clone()); + } + for s in scopes { + set.insert(s.clone()); + } + set.into_iter().collect() + } + (None, _) => parent.required_scopes.clone(), + }; + + ResolvedCredentialIntentPolicy { + intent: child.intent.clone(), + provider: child.provider.or(parent.provider), + runtime: child.runtime.or(parent.runtime), + secret: child.secret.clone().or_else(|| parent.secret.clone()), + required_scopes, + impersonation: child + .impersonation + .clone() + .or_else(|| parent.impersonation.clone()), + version_selector: child + .version_selector + .clone() + .or_else(|| parent.version_selector.clone()), + } +} + +/// Policy resolution/validation errors. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CredentialPolicyError { + UnknownProfile(String), + UnknownIntent { profile: String, intent: String }, + DuplicateProfile(String), + DuplicateIntent { profile: String, intent: String }, + InheritanceCycle(Vec), + MissingSecretBinding { profile: String, intent: String }, +} + +impl std::fmt::Display for CredentialPolicyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CredentialPolicyError::UnknownProfile(profile) => { + write!(f, "unknown credential policy profile '{profile}'") + } + CredentialPolicyError::UnknownIntent { profile, intent } => { + write!( + f, + "unknown intent policy '{intent}' in profile '{profile}'" + ) + } + CredentialPolicyError::DuplicateProfile(profile) => { + write!(f, "duplicate credential policy profile '{profile}'") + } + CredentialPolicyError::DuplicateIntent { profile, intent } => { + write!( + f, + "duplicate intent policy '{intent}' in profile '{profile}'" + ) + } + CredentialPolicyError::InheritanceCycle(cycle) => { + write!( + f, + "credential policy inheritance cycle: {}", + cycle.join(" -> ") + ) + } + CredentialPolicyError::MissingSecretBinding { profile, intent } => { + write!( + f, + "missing secret binding for intent '{intent}' in profile '{profile}'" + ) + } + } + } +} + +impl std::error::Error for CredentialPolicyError {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn inherited_profile_merges_defaults_and_intents() { + let spec = CredentialPolicySpec { + version: 0, + default_profile: Some("dev".to_string()), + profiles: vec![ + CredentialPolicyProfile { + name: "base".to_string(), + inherits_from: None, + defaults: CredentialPolicyDefaults { + provider: Some(CloudProviderKind::Gcp), + runtime: Some(CloudRuntimeKind::LocalDev), + impersonation: Some(ImpersonationPolicy::IfConfigured { + service_account: Some("base-sa@example.iam.gserviceaccount.com".to_string()), + }), + version_selector: Some(VersionSelector::Alias { + name: "active".to_string(), + }), + scope_merge: ScopeMergeMode::Replace, + }, + intents: vec![CredentialIntentPolicy { + intent: "github.gist.create".to_string(), + provider: None, + runtime: None, + secret: Some(SecretBinding { + name: "github-token".to_string(), + prefix: Some("dev-".to_string()), + delimiter: Some(String::new()), + }), + required_scopes: Some(vec!["gist:write".to_string()]), + scope_merge: None, + impersonation: None, + version_selector: None, + }], + }, + CredentialPolicyProfile { + name: "dev".to_string(), + inherits_from: Some("base".to_string()), + defaults: CredentialPolicyDefaults { + provider: None, + runtime: Some(CloudRuntimeKind::GitHubActions), + impersonation: None, + version_selector: Some(VersionSelector::Alias { + name: "dev-active".to_string(), + }), + scope_merge: ScopeMergeMode::Replace, + }, + intents: vec![CredentialIntentPolicy { + intent: "github.gist.create".to_string(), + provider: None, + runtime: None, + secret: None, + required_scopes: Some(vec![ + "gist:write".to_string(), + "repo".to_string(), + ]), + scope_merge: Some(ScopeMergeMode::Union), + impersonation: Some(ImpersonationPolicy::Always { + service_account: + "dev-secrets@example.iam.gserviceaccount.com".to_string(), + }), + version_selector: None, + }], + }, + ], + }; + + let resolved = spec + .resolve_intent_policy("dev", "github.gist.create") + .expect("intent policy should resolve"); + + assert_eq!(resolved.provider, Some(CloudProviderKind::Gcp)); + assert_eq!(resolved.runtime, Some(CloudRuntimeKind::GitHubActions)); + assert_eq!( + resolved.version_selector, + Some(VersionSelector::Alias { + name: "dev-active".to_string() + }) + ); + assert!(resolved.required_scopes.contains(&"gist:write".to_string())); + assert!(resolved.required_scopes.contains(&"repo".to_string())); + assert_eq!( + resolved.impersonation, + Some(ImpersonationPolicy::Always { + service_account: "dev-secrets@example.iam.gserviceaccount.com".to_string(), + }) + ); + } + + #[test] + fn detects_inheritance_cycle() { + let spec = CredentialPolicySpec { + version: 0, + default_profile: None, + profiles: vec![ + CredentialPolicyProfile { + name: "a".to_string(), + inherits_from: Some("b".to_string()), + defaults: CredentialPolicyDefaults::default(), + intents: vec![], + }, + CredentialPolicyProfile { + name: "b".to_string(), + inherits_from: Some("a".to_string()), + defaults: CredentialPolicyDefaults::default(), + intents: vec![], + }, + ], + }; + + let err = spec.resolve_profile("a").expect_err("should detect cycle"); + assert!(matches!(err, CredentialPolicyError::InheritanceCycle(_))); + } + + #[test] + fn resolve_intent_requires_secret_binding() { + let spec = CredentialPolicySpec { + version: 0, + default_profile: None, + profiles: vec![CredentialPolicyProfile { + name: "dev".to_string(), + inherits_from: None, + defaults: CredentialPolicyDefaults::default(), + intents: vec![CredentialIntentPolicy { + intent: "llm.chat.openai".to_string(), + provider: Some(CloudProviderKind::Gcp), + runtime: Some(CloudRuntimeKind::LocalDev), + secret: None, + required_scopes: Some(vec!["llm:chat_completion".to_string()]), + scope_merge: None, + impersonation: None, + version_selector: Some(VersionSelector::Alias { + name: "active".to_string(), + }), + }], + }], + }; + + let err = spec + .resolve_intent_policy("dev", "llm.chat.openai") + .expect_err("missing secret must fail"); + + assert!(matches!( + err, + CredentialPolicyError::MissingSecretBinding { .. } + )); + } +} diff --git a/core/ir/src/transport/mod.rs b/core/ir/src/transport/mod.rs index 9fc26088c5d..6a142abd4e1 100644 --- a/core/ir/src/transport/mod.rs +++ b/core/ir/src/transport/mod.rs @@ -43,6 +43,7 @@ pub mod ci; pub mod cli; pub mod cloud; pub mod credential; +pub mod credential_policy; pub mod file; pub mod gcp; pub mod gist; @@ -89,6 +90,19 @@ pub use gcp::{ }; pub use infra_scope::{GcpScope, InfraAccessLevel, InfraScope, InfraScopeType}; pub use credential::{AuthScheme, Credential, CredentialError, Secret, SecretSource}; +pub use credential_policy::{ + CredentialIntentPolicy, + CredentialPolicyDefaults, + CredentialPolicyError, + CredentialPolicyProfile, + CredentialPolicySpec, + ImpersonationPolicy, + ResolvedCredentialIntentPolicy, + ResolvedCredentialPolicyProfile, + ScopeMergeMode, + SecretBinding, + VersionSelector, +}; pub use file::{FileOp, FileRequest, FileResponse}; pub use gist::GistRequest; pub use git::GitRequest; diff --git a/gunbc-dag/src/bootstrap/graph_mock.rs b/gunbc-dag/src/bootstrap/graph_mock.rs index a08accd712b..50200704d39 100644 --- a/gunbc-dag/src/bootstrap/graph_mock.rs +++ b/gunbc-dag/src/bootstrap/graph_mock.rs @@ -177,11 +177,35 @@ pub fn bootstrap_mock_spec() -> MockSpec { Value::Str(".gitignore".into()), ) .input_mock("compare_makefile_content", "check_mode", Value::Bool(false)) + .input_mock( + "compare_makefile_content", + "response", + Value::Response(TransportResponse::File(FileResponse { + path: "Makefile".into(), + operation: FileOp::Read, + success: true, + content: Some("".into()), + exists: None, + error: None, + })), + ) .input_mock( "compare_gitignore_content", "check_mode", Value::Bool(false), ) + .input_mock( + "compare_gitignore_content", + "response", + Value::Response(TransportResponse::File(FileResponse { + path: ".gitignore".into(), + operation: FileOp::Read, + success: true, + content: Some("".into()), + exists: None, + error: None, + })), + ) // Resources: file locks for both outputs .resource_lock("fs:Makefile") .resource_lock("fs:.gitignore") diff --git a/gunbc-dag/src/credential_lifecycle.rs b/gunbc-dag/src/credential_lifecycle.rs index deff8b226eb..c76271eef82 100644 --- a/gunbc-dag/src/credential_lifecycle.rs +++ b/gunbc-dag/src/credential_lifecycle.rs @@ -137,6 +137,15 @@ pub fn github_credential_lifecycle_mock_spec() -> MockSpec { .output("skip", OutputMatcher::exact(Value::Bool(false))) .description("Prepare request builds GitHub REST rate_limit call"), ) + .node_example( + NodeExample::new("scope_preflight") + .input( + "required_scopes", + Value::str_list(vec!["gist:write".to_string()]), + ) + .output("scope_verified", OutputMatcher::exact(Value::Bool(true))) + .description("Scope preflight validates required scope declarations"), + ) .node_example( NodeExample::new("parse_status") .input( diff --git a/gunbc-dag/src/makegen/graph_mock.rs b/gunbc-dag/src/makegen/graph_mock.rs index d8bd97588b8..e7687e60fdc 100644 --- a/gunbc-dag/src/makegen/graph_mock.rs +++ b/gunbc-dag/src/makegen/graph_mock.rs @@ -124,6 +124,18 @@ pub fn makegen_mock_spec() -> MockSpec { Value::Str("Makefile".into()), ) .input_mock("compare_makegen_content", "check_mode", Value::Bool(false)) + .input_mock( + "compare_makegen_content", + "response", + Value::Response(TransportResponse::File(FileResponse { + path: "Makefile".into(), + operation: FileOp::Read, + success: true, + content: Some(mock_makefile_content()), + exists: None, + error: None, + })), + ) // Input expectations (via legacy API post-build) .expects_input("path", InputConstraint::Any) .expects_input("check_mode", InputConstraint::Any) diff --git a/gunbc-dag/src/pragma/graph_mock.rs b/gunbc-dag/src/pragma/graph_mock.rs index ccbd744c908..35e0a3b8740 100644 --- a/gunbc-dag/src/pragma/graph_mock.rs +++ b/gunbc-dag/src/pragma/graph_mock.rs @@ -210,12 +210,48 @@ pub fn pragma_mock_spec() -> MockSpec { Value::Str("tools/pragma-lint-policy.txt".into()), ) .input_mock("compare_clippy_content", "check_mode", Value::Bool(false)) + .input_mock( + "compare_clippy_content", + "response", + Value::Response(TransportResponse::File(FileResponse { + path: "clippy.toml".into(), + operation: FileOp::Read, + success: true, + content: Some("".into()), + exists: None, + error: None, + })), + ) .input_mock( "compare_allowlist_content", "check_mode", Value::Bool(false), ) + .input_mock( + "compare_allowlist_content", + "response", + Value::Response(TransportResponse::File(FileResponse { + path: "tools/disallowed-methods-allowlist.txt".into(), + operation: FileOp::Read, + success: true, + content: Some("".into()), + exists: None, + error: None, + })), + ) .input_mock("compare_policy_content", "check_mode", Value::Bool(false)) + .input_mock( + "compare_policy_content", + "response", + Value::Response(TransportResponse::File(FileResponse { + path: "tools/pragma-lint-policy.txt".into(), + operation: FileOp::Read, + success: true, + content: Some("".into()), + exists: None, + error: None, + })), + ) // Resources: file locks for all outputs .resource_lock("fs:clippy.toml") .resource_lock("fs:tools/disallowed-methods-allowlist.txt") diff --git a/gunbc-dag/src/testgen_dag/graph_mock.rs b/gunbc-dag/src/testgen_dag/graph_mock.rs index ed3b12190c8..c5c728bfe93 100644 --- a/gunbc-dag/src/testgen_dag/graph_mock.rs +++ b/gunbc-dag/src/testgen_dag/graph_mock.rs @@ -103,6 +103,18 @@ pub fn testgen_dag_mock_spec() -> MockSpec { "check_mode", Value::Bool(false), ) + .input_mock( + format!("compare_{}_content", name), + "response", + Value::Response(TransportResponse::File(FileResponse { + path: format!("{}/generated_tests.rs", name.replace('-', "_")), + operation: FileOp::Read, + success: true, + content: Some(format!("", name)), + exists: None, + error: None, + })), + ) .resource_lock(format!("fs:{}/generated_tests.rs", name.replace('-', "_"))); } diff --git a/gunbc-dag/src/workspace/subdags/gist.rs b/gunbc-dag/src/workspace/subdags/gist.rs index 2cf11b20a15..2f059506ba6 100644 --- a/gunbc-dag/src/workspace/subdags/gist.rs +++ b/gunbc-dag/src/workspace/subdags/gist.rs @@ -68,14 +68,14 @@ pub fn build_gist_subdag( /// Build a gist SubDag with explicit cloud config. /// /// When `cloud_config` is `Some`, it is used directly; when `None`, -/// `default_local_dev_config()` provides a sensible default. +/// `graph_cloud_config()` provides centralized profile-aware resolution. pub fn build_gist_subdag_with_config( mode: GistMode, extensions: Vec, create_gist: bool, cloud_config: Option, ) -> Node { - let config = cloud_config.unwrap_or_else(gunbc_lib_cloud_ops::default_local_dev_config); + let config = cloud_config.unwrap_or_else(gunbc_lib_cloud_ops::graph_cloud_config); let original = build_gist_graph_with_config(mode, extensions, create_gist, config) .expect("Gist graph should build"); let converted_dag = convert_dag(original, &convert_gist_op); diff --git a/gunbc-dag/tests/tool_registration.rs b/gunbc-dag/tests/tool_registration.rs index 9382133c458..119a68f8738 100644 --- a/gunbc-dag/tests/tool_registration.rs +++ b/gunbc-dag/tests/tool_registration.rs @@ -10,7 +10,7 @@ use std::path::Path; // and iter_tool_targets() would return an empty iterator. use gunbc_clippy::clippy_tool; use gunbc_deps::deps_tool; -use gunbc_gist::{gist_auth_doctor_tool, gist_diff_tool, gist_recent_tool, gist_snapshot_tool}; +use gunbc_gist::{gist_diff_tool, gist_recent_tool, gist_snapshot_tool}; use gunbc_lib_review::review_tool; // These are in gunbc-dag itself (same binary), but reference for completeness. use gunbc_dag::bootstrap::bootstrap_tool; @@ -24,7 +24,6 @@ fn derive_tool_defs_matches_inventory() { let _: fn() = gist_snapshot_tool; let _: fn() = gist_diff_tool; let _: fn() = gist_recent_tool; - let _: fn() = gist_auth_doctor_tool; let _: fn() = deps_tool; let _: fn() = review_tool; let _: fn() = makegen_tool; @@ -46,7 +45,6 @@ fn derive_tool_defs_matches_inventory() { "clippy", "deps", "gist", - "gist-auth-doctor", "gist-diff", "gist-recent", "makegen", diff --git a/lib/aws-ops/src/graph.rs b/lib/aws-ops/src/graph.rs index 1616ec06ebc..73ae7bb835b 100644 --- a/lib/aws-ops/src/graph.rs +++ b/lib/aws-ops/src/graph.rs @@ -2,7 +2,7 @@ use crate::ops::AwsOps; use gunbc_exec::{ExecError, Executable}; -use gunbc_ir::build::{optional, port}; +use gunbc_ir::build::{list, optional, port}; use gunbc_ir::{Dag, DagBuilder, Node, Value}; use std::collections::HashMap; @@ -34,6 +34,7 @@ pub fn build_aws_secrets_manager_credential_graph() -> Dag Dag { port("scheme", "String"), optional("header_name", "OptionalString"), port("source_id", "String"), + list("required_scopes", "String"), optional("lifetime_seconds", "OptionalInt"), optional("request_url", "OptionalString"), optional("request_token", "OptionalString"), diff --git a/lib/cloud-ops/src/auth_doctor.rs b/lib/cloud-ops/src/auth_doctor.rs deleted file mode 100644 index 6b6948d15c1..00000000000 --- a/lib/cloud-ops/src/auth_doctor.rs +++ /dev/null @@ -1,193 +0,0 @@ -//! Provider-neutral cloud auth doctor. -//! -//! Builds a diagnostic DAG that any credentialed tool can use to report -//! environment readiness. The tool passes its `CredentialIntent` to -//! parameterize the report. - -use crate::env_requirements::{collect_missing_requirements, detect_cloud_env_requirements}; -use crate::env_status::CloudEnvStatus; -use gunbc_exec::{require_str, ExecError, Executable, OutputMap}; -use gunbc_ir::build::{list, port}; -use gunbc_ir::transport::cloud::CloudRuntimeKind; -use gunbc_ir::transport::scope::CredentialIntent; -use gunbc_ir::{BuilderError, Dag, DagBuilder, Node, Value}; -use std::collections::HashMap; - -/// Op enum for the cloud auth doctor graph. -#[derive(Debug, Clone)] -pub enum CloudAuthDoctorOp { - /// Environment status node. - CloudStatus(CloudEnvStatus), - /// Build the diagnostic report. - BuildReport { - intent: CredentialIntent, - runtime_hint: Option, - }, -} - -impl Executable for CloudAuthDoctorOp { - fn execute(&self, inputs: HashMap) -> Result, ExecError> { - match self { - CloudAuthDoctorOp::CloudStatus(op) => op.execute(inputs), - CloudAuthDoctorOp::BuildReport { - intent, - runtime_hint, - } => execute_build_report(inputs, intent, runtime_hint.as_deref()), - } - } -} - -fn execute_build_report( - inputs: HashMap, - intent: &CredentialIntent, - runtime_hint: Option<&str>, -) -> Result, ExecError> { - let status = require_str(&inputs, "status")?; - - let mut env_req = detect_cloud_env_requirements(); - if let Some(hint) = runtime_hint { - let runtime = CloudRuntimeKind::parse(hint) - .ok_or_else(|| ExecError::new(format!("invalid runtime_hint '{hint}'")))?; - env_req = crate::env_requirements::requirements_for(env_req.provider, runtime); - } - - intent - .validate() - .map_err(|e| ExecError::new(format!("invalid scope contract: {e}")))?; - - let required_env: Vec = env_req.required.iter().map(|v| (*v).to_string()).collect(); - let missing = collect_missing_requirements(&env_req); - let missing_env: Vec = missing - .missing_required - .iter() - .map(|v| (*v).to_string()) - .collect(); - - let required_any_of: Vec = env_req - .required_any_of - .iter() - .map(|group| group.join(" | ")) - .collect(); - let missing_any_of: Vec = missing - .missing_any_of - .iter() - .map(|group| group.join(" | ")) - .collect(); - - let secret_prefix = - std::env::var("GCP_SECRETS_PREFIX").unwrap_or_else(|_| "".to_string()); - let secret_name = format!("{secret_prefix}{}", intent.service); - - let local_setup = if matches!(env_req.runtime, CloudRuntimeKind::LocalDev) { - "login-on-demand via credential upsert (manual fallback: gcloud auth login --update-adc)" - } else { - "n/a" - }; - - let ready = missing_env.is_empty() && missing_any_of.is_empty(); - - OutputMap::new() - .str("status", status) - .str("provider", env_req.provider.as_str()) - .str("runtime", env_req.runtime.as_str()) - .str_list("required_env", required_env) - .str_list("missing_env", missing_env) - .str_list("required_any_of", required_any_of) - .str_list("missing_any_of", missing_any_of) - .str("service", &intent.service) - .str_list("required_scopes", intent.required_scopes.clone()) - .str("secret_name", secret_name) - .str( - "acquisition_flow", - "resolve_auth_contract -> cloud_env -> bind_secret -> cloud_credential(local: check_auth -> create_auth_if_needed -> resolve_token) -> execute(res:credential)", - ) - .str("local_setup", local_setup) - .bool("ready", ready) - .ok() -} - -/// Build a provider-neutral auth doctor graph for the given credential intent. -/// -/// The `intent` specifies which service and scopes are being diagnosed. -/// The `runtime_hint` allows overriding the detected runtime (e.g., from CLI args). -pub fn build_cloud_auth_doctor_graph( - intent: CredentialIntent, - runtime_hint: Option, -) -> Result, BuilderError> { - let mut builder: DagBuilder = DagBuilder::new(); - - let cloud_status = builder.add_root_node(Node::opaque( - "cloud_status", - vec![], - vec![port("status", "String")], - CloudAuthDoctorOp::CloudStatus(CloudEnvStatus::new()), - ))?; - - let auth_report = builder.add_node_after( - Node::opaque( - "auth_report", - vec![port("status", "String")], - vec![ - port("status", "String"), - port("provider", "String"), - port("runtime", "String"), - list("required_env", "String"), - list("missing_env", "String"), - list("required_any_of", "String"), - list("missing_any_of", "String"), - port("service", "String"), - list("required_scopes", "String"), - port("secret_name", "String"), - port("acquisition_flow", "String"), - port("local_setup", "String"), - port("ready", "Bool"), - ], - CloudAuthDoctorOp::BuildReport { - intent, - runtime_hint, - }, - ), - &cloud_status, - )?; - - builder.add_edge(cloud_status.out("status"), auth_report.in_port("status"))?; - - Ok(builder.build()) -} - -#[cfg(test)] -mod tests { - use super::*; - use gunbc_exec::{execute_with_mode, ExecutionMode}; - - #[test] - fn test_cloud_auth_doctor_runs_with_gist_intent() { - let intent = CredentialIntent::new("github", "github", "bearer") - .with_required_scopes(["gist:write"]); - let dag = build_cloud_auth_doctor_graph(intent, None) - .expect("auth doctor graph should build"); - let log = execute_with_mode(&dag, ExecutionMode::DryRun(Default::default())) - .expect("auth doctor graph should execute"); - let report = log.get("auth_report").expect("auth_report should run"); - assert!(report.outputs.contains_key("required_scopes")); - assert_eq!( - report.outputs.get("service"), - Some(&Value::Str("github".to_string())) - ); - } - - #[test] - fn test_cloud_auth_doctor_runs_with_llm_intent() { - let intent = CredentialIntent::new("openai", "openai", "bearer") - .with_required_scopes(["llm:chat_completion"]); - let dag = build_cloud_auth_doctor_graph(intent, None) - .expect("auth doctor graph should build"); - let log = execute_with_mode(&dag, ExecutionMode::DryRun(Default::default())) - .expect("auth doctor graph should execute"); - let report = log.get("auth_report").expect("auth_report should run"); - assert_eq!( - report.outputs.get("service"), - Some(&Value::Str("openai".to_string())) - ); - } -} diff --git a/lib/cloud-ops/src/config_loader.rs b/lib/cloud-ops/src/config_loader.rs index 0bf0f1d4015..213155209bc 100644 --- a/lib/cloud-ops/src/config_loader.rs +++ b/lib/cloud-ops/src/config_loader.rs @@ -11,6 +11,19 @@ use gunbc_ir::transport::cloud::{ CloudSecretRef, }; +const ENV_CONFIG_JSON: &str = "GUNBC_CLOUD_CONFIG_JSON"; +const ENV_CONFIG_TOML: &str = "GUNBC_CLOUD_CONFIG_TOML"; +const ENV_PROFILE: &str = "GUNBC_CLOUD_PROFILE"; +const ENV_NAMESPACE: &str = "GUNBC_CLOUD_NAMESPACE"; +const ENV_SECRET_VERSION: &str = "GUNBC_CLOUD_SECRET_VERSION"; +const ENV_CONFIG_REQUIRED: &str = "GUNBC_CLOUD_CONFIG_REQUIRED"; + +const LEGACY_ENV_WIF_PROVIDER: &str = "GCP_WIF_PROVIDER"; +const LEGACY_ENV_SECRETS_PROJECT: &str = "GCP_SECRETS_PROJECT"; +const LEGACY_ENV_SECRETS_PREFIX: &str = "GCP_SECRETS_PREFIX"; +const LEGACY_ENV_SECRETS_SA: &str = "GCP_SECRETS_SA"; +const LEGACY_ENV_IMPERSONATE_SA: &str = "GCP_SECRETS_IMPERSONATE_SA"; + // --------------------------------------------------------------------------- // TOML parser (minimal, avoids pulling in toml crate) // --------------------------------------------------------------------------- @@ -122,6 +135,130 @@ pub fn detect_runtime() -> CloudRuntimeKind { CloudRuntimeKind::LocalDev } +// --------------------------------------------------------------------------- +// Graph config resolution (centralized precedence) +// --------------------------------------------------------------------------- + +/// Resolve graph cloud config using deterministic precedence. +/// +/// Precedence: +/// 1) `GUNBC_CLOUD_CONFIG_JSON` (serialized `CloudSecretConfig`) +/// 2) `GUNBC_CLOUD_CONFIG_TOML` (+ namespace/profile selection) +/// 3) Legacy env model (`GCP_*`) +/// +/// If no source is configured, this returns an error. +pub fn resolve_graph_cloud_config() -> Result { + if let Some(raw_json) = env_nonempty(ENV_CONFIG_JSON) { + let mut config: CloudSecretConfig = serde_json::from_str(&raw_json).map_err(|e| { + format!("{ENV_CONFIG_JSON} must contain valid CloudSecretConfig JSON: {e}") + })?; + if let Some(version) = env_nonempty(ENV_SECRET_VERSION) { + config.secret.version = Some(version); + } + return Ok(config); + } + + if let Some(raw_toml) = env_nonempty(ENV_CONFIG_TOML) { + let spec = parse_config_toml(&raw_toml) + .map_err(|e| format!("failed to parse {ENV_CONFIG_TOML}: {e}"))?; + let runtime = runtime_with_override(); + let namespace = env_nonempty(ENV_NAMESPACE) + .or_else(|| env_nonempty(ENV_PROFILE)) + .or_else(|| spec.default_namespace.clone()) + .ok_or_else(|| { + format!( + "{ENV_CONFIG_TOML} is set but no namespace/profile resolved; set {ENV_NAMESPACE} or {ENV_PROFILE}, or include default_namespace" + ) + })?; + + let mut config = spec + .to_secret_config(&namespace, runtime, "") + .ok_or_else(|| format!("namespace '{namespace}' could not be resolved from config"))?; + if let Some(version) = env_nonempty(ENV_SECRET_VERSION) { + config.secret.version = Some(version); + } + return Ok(config); + } + + if let Some(config) = resolve_legacy_gcp_env_config() { + return Ok(config); + } + + Err(format!( + "no cloud config source found (expected {ENV_CONFIG_JSON} or {ENV_CONFIG_TOML}, or legacy {LEGACY_ENV_SECRETS_PROJECT}/{LEGACY_ENV_SECRETS_PREFIX})" + )) +} + +/// Resolve graph cloud config with compatibility fallback. +/// +/// When config sources are absent, this falls back to `default_local_dev_config()` +/// unless `GUNBC_CLOUD_CONFIG_REQUIRED=1|true`. +pub fn graph_cloud_config() -> CloudSecretConfig { + match resolve_graph_cloud_config() { + Ok(config) => config, + Err(err) => { + if env_truthy(ENV_CONFIG_REQUIRED) { + panic!("cloud config resolution failed: {err}"); + } + default_local_dev_config() + } + } +} + +fn env_nonempty(name: &str) -> Option { + std::env::var(name) + .ok() + .map(|v| v.trim().to_string()) + .filter(|v| !v.is_empty()) +} + +fn env_truthy(name: &str) -> bool { + env_nonempty(name) + .map(|v| { + matches!( + v.as_str(), + "1" | "true" | "TRUE" | "True" | "yes" | "YES" | "on" | "ON" + ) + }) + .unwrap_or(false) +} + +fn runtime_with_override() -> CloudRuntimeKind { + env_nonempty("CLOUD_RUNTIME") + .and_then(|v| CloudRuntimeKind::parse(&v)) + .unwrap_or_else(detect_runtime) +} + +fn resolve_legacy_gcp_env_config() -> Option { + if let Some(provider) = + env_nonempty("CLOUD_PROVIDER").and_then(|v| CloudProviderKind::parse(&v)) + { + if provider != CloudProviderKind::Gcp { + return None; + } + } + + let project = env_nonempty(LEGACY_ENV_SECRETS_PROJECT)?; + let prefix = env_nonempty(LEGACY_ENV_SECRETS_PREFIX)?; + let audience = env_nonempty(LEGACY_ENV_WIF_PROVIDER).unwrap_or_else(|| "local-dev".to_string()); + let runtime = runtime_with_override(); + + Some(CloudSecretConfig { + provider: CloudProviderKind::Gcp, + runtime, + audience, + project_or_account: project, + secret: CloudSecretRef { + prefix, + name: String::new(), + delimiter: String::new(), + version: env_nonempty(ENV_SECRET_VERSION), + }, + service_account_or_role: env_nonempty(LEGACY_ENV_SECRETS_SA), + impersonate_account_or_role: env_nonempty(LEGACY_ENV_IMPERSONATE_SA), + }) +} + // --------------------------------------------------------------------------- // Config resolution // --------------------------------------------------------------------------- @@ -190,6 +327,7 @@ pub fn default_local_dev_config() -> CloudSecretConfig { #[cfg(test)] mod tests { use super::*; + use std::sync::{Mutex, OnceLock}; const SAMPLE_TOML: &str = r#" default_namespace = "dev" @@ -285,4 +423,124 @@ service_account = "gunbai-prod-secrets@gunbai-secrets.iam.gserviceaccount.com" assert!(spec.namespaces.is_empty()); assert!(spec.default_namespace.is_none()); } + + #[test] + fn resolve_graph_cloud_config_prefers_json_over_toml() { + with_env_lock(|| { + let json_config = CloudSecretConfig { + provider: CloudProviderKind::Gcp, + runtime: CloudRuntimeKind::LocalDev, + audience: "json-audience".to_string(), + project_or_account: "json-project".to_string(), + secret: CloudSecretRef { + prefix: "json-".to_string(), + name: String::new(), + delimiter: String::new(), + version: None, + }, + service_account_or_role: Some("json-sa@example".to_string()), + impersonate_account_or_role: None, + }; + std::env::set_var( + ENV_CONFIG_JSON, + serde_json::to_string(&json_config).expect("serialize json config"), + ); + std::env::set_var(ENV_CONFIG_TOML, SAMPLE_TOML); + std::env::set_var(ENV_PROFILE, "prod"); + + let resolved = resolve_graph_cloud_config().expect("json config should resolve"); + assert_eq!(resolved.project_or_account, "json-project"); + assert_eq!(resolved.audience, "json-audience"); + }); + } + + #[test] + fn resolve_graph_cloud_config_uses_toml_profile_when_present() { + with_env_lock(|| { + std::env::set_var(ENV_CONFIG_TOML, SAMPLE_TOML); + std::env::set_var(ENV_PROFILE, "prod"); + + let resolved = resolve_graph_cloud_config().expect("toml config should resolve"); + assert_eq!(resolved.project_or_account, "gunbai-secrets"); + assert_eq!(resolved.secret.prefix, "prod-"); + assert_eq!( + resolved.service_account_or_role, + Some("gunbai-prod-secrets@gunbai-secrets.iam.gserviceaccount.com".to_string()) + ); + }); + } + + #[test] + fn resolve_graph_cloud_config_uses_legacy_env_when_no_spec_source() { + with_env_lock(|| { + std::env::set_var(LEGACY_ENV_SECRETS_PROJECT, "legacy-project"); + std::env::set_var(LEGACY_ENV_SECRETS_PREFIX, "legacy-"); + std::env::set_var(LEGACY_ENV_WIF_PROVIDER, "legacy-audience"); + std::env::set_var(LEGACY_ENV_SECRETS_SA, "legacy-sa@example"); + + let resolved = resolve_graph_cloud_config().expect("legacy env should resolve"); + assert_eq!(resolved.project_or_account, "legacy-project"); + assert_eq!(resolved.secret.prefix, "legacy-"); + assert_eq!(resolved.audience, "legacy-audience"); + assert_eq!( + resolved.service_account_or_role, + Some("legacy-sa@example".to_string()) + ); + }); + } + + #[test] + fn graph_cloud_config_falls_back_to_default_when_unconfigured() { + with_env_lock(|| { + let resolved = graph_cloud_config(); + assert_eq!(resolved.project_or_account, "gunbai-secrets"); + assert_eq!(resolved.secret.prefix, "dev-"); + }); + } + + #[test] + #[should_panic(expected = "cloud config resolution failed")] + fn graph_cloud_config_panics_when_required_flag_is_set() { + with_env_lock(|| { + std::env::set_var(ENV_CONFIG_REQUIRED, "true"); + let _ = graph_cloud_config(); + }); + } + + fn with_env_lock(f: F) + where + F: FnOnce() + std::panic::UnwindSafe, + { + static ENV_LOCK: OnceLock> = OnceLock::new(); + let _guard = ENV_LOCK + .get_or_init(|| Mutex::new(())) + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + clear_cloud_config_env(); + let result = std::panic::catch_unwind(f); + clear_cloud_config_env(); + if let Err(panic) = result { + std::panic::resume_unwind(panic); + } + } + + fn clear_cloud_config_env() { + for key in [ + ENV_CONFIG_JSON, + ENV_CONFIG_TOML, + ENV_PROFILE, + ENV_NAMESPACE, + ENV_SECRET_VERSION, + ENV_CONFIG_REQUIRED, + "CLOUD_PROVIDER", + "CLOUD_RUNTIME", + LEGACY_ENV_WIF_PROVIDER, + LEGACY_ENV_SECRETS_PROJECT, + LEGACY_ENV_SECRETS_PREFIX, + LEGACY_ENV_SECRETS_SA, + LEGACY_ENV_IMPERSONATE_SA, + ] { + std::env::remove_var(key); + } + } } diff --git a/lib/cloud-ops/src/config_resource.rs b/lib/cloud-ops/src/config_resource.rs index 1a2c8be7c34..7bd8ad071d6 100644 --- a/lib/cloud-ops/src/config_resource.rs +++ b/lib/cloud-ops/src/config_resource.rs @@ -119,10 +119,7 @@ mod tests { } impl ResourceIo for TestIo { - fn read_file( - &self, - path: &std::path::Path, - ) -> Result, ResourceError> { + fn read_file(&self, path: &std::path::Path) -> Result, ResourceError> { self.files.borrow().get(path).cloned().ok_or_else(|| { ResourceError::Io(std::io::Error::other(format!( "file not found: {}", @@ -131,28 +128,18 @@ mod tests { }) } - fn write_file( - &self, - path: &std::path::Path, - contents: &[u8], - ) -> Result<(), ResourceError> { + fn write_file(&self, path: &std::path::Path, contents: &[u8]) -> Result<(), ResourceError> { self.files .borrow_mut() .insert(path.to_path_buf(), contents.to_vec()); Ok(()) } - fn file_exists( - &self, - path: &std::path::Path, - ) -> Result { + fn file_exists(&self, path: &std::path::Path) -> Result { Ok(self.files.borrow().contains_key(path)) } - fn glob_paths( - &self, - _pattern: &str, - ) -> Result, ResourceError> { + fn glob_paths(&self, _pattern: &str) -> Result, ResourceError> { Ok(vec![]) } @@ -164,10 +151,7 @@ mod tests { Err(ResourceError::Io(std::io::Error::other("not supported"))) } - fn file_mtime( - &self, - _path: &std::path::Path, - ) -> Result { + fn file_mtime(&self, _path: &std::path::Path) -> Result { Ok(SystemTime::now()) } } diff --git a/lib/cloud-ops/src/env_requirements.rs b/lib/cloud-ops/src/env_requirements.rs index 1b686fd21dd..caf092590c3 100644 --- a/lib/cloud-ops/src/env_requirements.rs +++ b/lib/cloud-ops/src/env_requirements.rs @@ -173,7 +173,10 @@ pub fn format_missing_requirements_message( ) -> String { let mut parts = Vec::new(); if !missing.missing_required.is_empty() { - parts.push(format!("provider-runtime required: {}", missing.missing_required.join(", "))); + parts.push(format!( + "provider-runtime required: {}", + missing.missing_required.join(", ") + )); } if !missing.missing_any_of.is_empty() { let groups = missing @@ -186,11 +189,10 @@ pub fn format_missing_requirements_message( } format!( - "missing cloud environment for {}/{}: {}; selectors: CLOUD_PROVIDER, CLOUD_RUNTIME; hint: run `make gist-auth-doctor RUNTIME={}`", + "missing cloud environment for {}/{}: {}; selectors: CLOUD_PROVIDER, CLOUD_RUNTIME", req.provider.as_str(), req.runtime.as_str(), parts.join("; "), - req.runtime.as_str(), ) } diff --git a/lib/cloud-ops/src/github_credential_graph.rs b/lib/cloud-ops/src/github_credential_graph.rs index 0eed4e760bb..4e6c892e897 100644 --- a/lib/cloud-ops/src/github_credential_graph.rs +++ b/lib/cloud-ops/src/github_credential_graph.rs @@ -3,11 +3,13 @@ //! Resolves GitHub credentials via the cloud secret manager and performs //! a minimal GitHub API call to validate the token. -use crate::config_loader::default_local_dev_config; -use crate::graph::{build_cloud_secret_manager_credential_graph_from_config, CloudSecretManagerGraphOp}; +use crate::config_loader::graph_cloud_config; +use crate::graph::{ + build_cloud_secret_manager_credential_graph_from_config, CloudSecretManagerGraphOp, +}; use crate::ops::CloudOps; use gunbc_exec::{require_response, ExecError, Executable, OutputMap}; -use gunbc_ir::build::{optional, port, resource}; +use gunbc_ir::build::{list, optional, port, resource}; use gunbc_ir::transport::github::api::github_rest_request; use gunbc_ir::transport::{TransportRequest, TransportResponse}; use gunbc_ir::{ @@ -33,6 +35,7 @@ impl Executable for GitHubCredentialOps { .str("service", "github") .str("scheme", "bearer") .str("header_name", "") + .str_list("required_scopes", vec!["github:api".to_string()]) .bool("interactive_allowed", true) .ok(), GitHubCredentialOps::PrepareRateLimit => { @@ -83,7 +86,7 @@ impl Executable for GitHubCredentialGraphOp { builder = "build_github_credential_graph()" )] pub fn build_github_credential_graph() -> Dag { - let config = default_local_dev_config(); + let config = graph_cloud_config(); let mut builder: DagBuilder = DagBuilder::new(); // Cloud environment — pre-resolved config (no env var reads). @@ -113,6 +116,7 @@ pub fn build_github_credential_graph() -> Dag { port("service", "String"), port("scheme", "String"), port("header_name", "String"), + list("required_scopes", "String"), port("interactive_allowed", "Bool"), ], GitHubCredentialGraphOp::GitHub(GitHubCredentialOps::ResolveAuth), @@ -146,7 +150,9 @@ pub fn build_github_credential_graph() -> Dag { .expect("resolve_auth.service -> bind_secret.service"); // Cloud credential acquisition graph — dispatched from config. - let cloud_subdag = lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config(&config)); + let cloud_subdag = lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config( + &config, + )); let cloud_credential = builder .add_node_after(Node::subdag("cloud_credential", cloud_subdag), &bind_secret) .expect("cloud_credential node"); @@ -181,6 +187,12 @@ pub fn build_github_credential_graph() -> Dag { cloud_credential.in_port("interactive_allowed"), ) .expect("resolve_auth.interactive_allowed -> cloud_credential.interactive_allowed"); + builder + .add_edge( + resolve_auth.out("required_scopes"), + cloud_credential.in_port("required_scopes"), + ) + .expect("resolve_auth.required_scopes -> cloud_credential.required_scopes"); builder .add_edge( cloud_env.out("request_url"), @@ -207,6 +219,27 @@ pub fn build_github_credential_graph() -> Dag { ) .expect("prepare_request node"); + // Scope preflight: fail fast on invalid/missing required scope declarations. + let scope_preflight = builder + .add_node_after( + Node::opaque( + "scope_preflight", + vec![list("required_scopes", "String")], + vec![port("scope_verified", "Bool")], + GitHubCredentialGraphOp::Cloud(CloudSecretManagerGraphOp::Cloud( + CloudOps::ScopePreflight, + )), + ), + &resolve_auth, + ) + .expect("scope_preflight node"); + builder + .add_edge( + resolve_auth.out("required_scopes"), + scope_preflight.in_port("required_scopes"), + ) + .expect("resolve_auth.required_scopes -> scope_preflight.required_scopes"); + // Execute transport + ParseStatus. let triplet = add_transport_execute_parse_named_with_passthrough( &mut builder, @@ -214,13 +247,22 @@ pub fn build_github_credential_graph() -> Dag { "execute", "parse_status", vec![], - vec![resource("credential", "Credential", AccessMode::Read)], + vec![ + optional("scope_verified", "OptionalBool"), + resource("credential", "Credential", AccessMode::Read), + ], vec![port("status", "Int"), port("ok", "Bool")], GitHubCredentialGraphOp::GitHub(GitHubCredentialOps::ParseStatus), GitHubCredentialGraphOp::Transport(TransportOps::Execute), Some(&cloud_credential), ) .expect("transport triplet"); + builder + .add_edge( + scope_preflight.out("scope_verified"), + triplet.execute.in_port("scope_verified"), + ) + .expect("scope_preflight.scope_verified -> execute.scope_verified"); builder .add_edge( diff --git a/lib/cloud-ops/src/graph.rs b/lib/cloud-ops/src/graph.rs index d7604cc2e6a..e05f4d6e2f1 100644 --- a/lib/cloud-ops/src/graph.rs +++ b/lib/cloud-ops/src/graph.rs @@ -2,7 +2,7 @@ use crate::ops::CloudOps; use gunbc_exec::{ExecError, Executable}; -use gunbc_ir::build::{optional, port}; +use gunbc_ir::build::{list, optional, port}; use gunbc_ir::transport::cloud::{CloudProviderKind, CloudRuntimeKind, CloudSecretConfig}; use gunbc_ir::{Dag, DagBuilder, Node, Value}; use gunbc_lib_aws_ops::{ @@ -214,6 +214,7 @@ fn build_cloud_secret_manager_credential_graph_gcp( port("scheme", "String"), optional("header_name", "OptionalString"), port("source_id", "String"), + list("required_scopes", "String"), optional("lifetime_seconds", "OptionalInt"), ]; // interactive_allowed is only needed for non-local runtimes where @@ -246,6 +247,7 @@ fn build_cloud_secret_manager_credential_graph_gcp( port("scheme", "String"), optional("header_name", "OptionalString"), port("source_id", "String"), + list("required_scopes", "String"), optional("lifetime_seconds", "OptionalInt"), // interactive_allowed is accepted as input for all runtimes // (parent graphs always wire it), but only OUTPUT for @@ -340,6 +342,12 @@ fn build_cloud_secret_manager_credential_graph_gcp( builder .add_edge(map_inputs.out("source_id"), gcp_node.in_port("source_id")) .expect("map_gcp_inputs.source_id -> gcp_wif_secret.source_id"); + builder + .add_edge( + map_inputs.out("required_scopes"), + gcp_node.in_port("required_scopes"), + ) + .expect("map_gcp_inputs.required_scopes -> gcp_wif_secret.required_scopes"); builder .add_edge( map_inputs.out("lifetime_seconds"), diff --git a/lib/cloud-ops/src/lib.rs b/lib/cloud-ops/src/lib.rs index a48353db7fc..4fa3ab328a0 100644 --- a/lib/cloud-ops/src/lib.rs +++ b/lib/cloud-ops/src/lib.rs @@ -3,7 +3,6 @@ //! This crate stitches provider-specific secret manager DAGs together behind a //! stable interface so callers can switch providers without reworking DAG shapes. -pub mod auth_doctor; pub mod config_loader; pub mod config_resource; pub mod env_requirements; @@ -12,7 +11,7 @@ mod github_credential_graph; mod graph; mod ops; -pub use config_loader::default_local_dev_config; +pub use config_loader::{default_local_dev_config, graph_cloud_config, resolve_graph_cloud_config}; pub use env_requirements::{ aws_github_actions_env_stub, azure_github_actions_env_stub, cloud_env_matrix, collect_missing_requirements, detect_cloud_env_requirements, detect_provider_runtime, @@ -33,8 +32,7 @@ pub use graph::{ build_cloud_secret_manager_upsert_graph_from_config, build_cloud_secret_manager_upsert_graph_gcp_github, build_cloud_secret_manager_upsert_graph_gcp_local, - build_cloud_secret_manager_upsert_graph_gcp_metadata, - CloudSecretManagerGraphOp, + build_cloud_secret_manager_upsert_graph_gcp_metadata, CloudSecretManagerGraphOp, }; pub use ops::CloudOps; @@ -59,8 +57,6 @@ pub use ops::CloudOps; /// )), /// )); /// ``` -pub fn const_cloud_config( - config: gunbc_ir::transport::cloud::CloudSecretConfig, -) -> CloudOps { +pub fn const_cloud_config(config: gunbc_ir::transport::cloud::CloudSecretConfig) -> CloudOps { CloudOps::ConstCloudConfig { config } } diff --git a/lib/cloud-ops/src/ops.rs b/lib/cloud-ops/src/ops.rs index c9f66fbb02f..1c5e17b2302 100644 --- a/lib/cloud-ops/src/ops.rs +++ b/lib/cloud-ops/src/ops.rs @@ -1,6 +1,8 @@ //! Cloud configuration ops. -use gunbc_exec::{optional_str_strict, require_str, ExecError, Executable, OutputMap}; +use gunbc_exec::{ + optional_str_list_strict, optional_str_strict, require_str, ExecError, Executable, OutputMap, +}; use gunbc_ir::transport::cloud::{CloudProviderKind, CloudRuntimeKind, CloudSecretConfig}; use gunbc_ir::Value; use serde::{Deserialize, Serialize}; @@ -21,9 +23,13 @@ pub enum CloudOps { /// Replacement for CloudEnv: takes a serialized config and emits /// the same outputs (config, request_url, request_token) without /// reading any environment variables. - ConstCloudConfig { - config: CloudSecretConfig, - }, + ConstCloudConfig { config: CloudSecretConfig }, + /// Validate declared required scopes before business transport execution. + /// + /// This op is intentionally provider-neutral. It validates that + /// `required_scopes` (when provided) are syntactically canonical. + /// It emits `scope_verified=true` on success and errors on invalid scopes. + ScopePreflight, } impl Executable for CloudOps { @@ -94,34 +100,30 @@ impl Executable for CloudOps { let audience = require_str(&inputs, "audience")?; let project = require_str(&inputs, "project_or_account")?; let secret = require_str(&inputs, "secret")?; + let required_scopes = + optional_str_list_strict(&inputs, "required_scopes")?.unwrap_or_default(); - let service_account = match inputs + let service_account = inputs .get("impersonate_account_or_role") .and_then(Value::as_str) .map(|s| s.trim()) .filter(|s| !s.is_empty()) - { - Some(value) => value.to_string(), - None => match inputs - .get("service_account_or_role") - .and_then(Value::as_str) - .map(|s| s.trim()) - .filter(|s| !s.is_empty()) - { - Some(value) => value.to_string(), - None => { - return Err(ExecError::new( - "missing service_account_or_role for gcp config", - )) - } - }, - }; + .or_else(|| { + inputs + .get("service_account_or_role") + .and_then(Value::as_str) + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + }) + .unwrap_or("") + .to_string(); let mut out = OutputMap::new() .str("audience", audience) .str("project", project) .str("secret", secret) .str("service_account", service_account) + .str_list("required_scopes", required_scopes) .ok()?; if let Some(version) = inputs.get("version").and_then(Value::as_str) { @@ -176,10 +178,10 @@ impl Executable for CloudOps { // These are still from env because they're ephemeral per-job values, // not infrastructure configuration. if matches!(config.runtime, CloudRuntimeKind::GitHubActions) { - let request_url = std::env::var("ACTIONS_ID_TOKEN_REQUEST_URL") - .unwrap_or_default(); - let request_token = std::env::var("ACTIONS_ID_TOKEN_REQUEST_TOKEN") - .unwrap_or_default(); + let request_url = + std::env::var("ACTIONS_ID_TOKEN_REQUEST_URL").unwrap_or_default(); + let request_token = + std::env::var("ACTIONS_ID_TOKEN_REQUEST_TOKEN").unwrap_or_default(); out = out .str("request_url", &request_url) .str("request_token", &request_token); @@ -187,6 +189,16 @@ impl Executable for CloudOps { out.ok() } + CloudOps::ScopePreflight => { + let required_scopes = + optional_str_list_strict(&inputs, "required_scopes")?.unwrap_or_default(); + + for scope in &required_scopes { + validate_scope_id(scope)?; + } + + OutputMap::new().bool("scope_verified", true).ok() + } CloudOps::MapToGcpSecretInputs { runtime } => { let provider = require_str(&inputs, "provider")?; if CloudProviderKind::parse(provider) != Some(CloudProviderKind::Gcp) { @@ -207,27 +219,20 @@ impl Executable for CloudOps { let project = require_str(&inputs, "project_or_account")?; let secret = require_str(&inputs, "secret")?; - let service_account = match inputs + let service_account = inputs .get("impersonate_account_or_role") .and_then(Value::as_str) .map(|s| s.trim()) .filter(|s| !s.is_empty()) - { - Some(value) => value.to_string(), - None => match inputs - .get("service_account_or_role") - .and_then(Value::as_str) - .map(|s| s.trim()) - .filter(|s| !s.is_empty()) - { - Some(value) => value.to_string(), - None => { - return Err(ExecError::new( - "missing service_account_or_role for gcp config", - )) - } - }, - }; + .or_else(|| { + inputs + .get("service_account_or_role") + .and_then(Value::as_str) + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + }) + .unwrap_or("") + .to_string(); let mut out = OutputMap::new() .str("audience", audience) @@ -270,3 +275,180 @@ impl Executable for CloudOps { } } } + +fn validate_scope_id(scope: &str) -> Result<(), ExecError> { + if scope.trim() != scope { + return Err(ExecError::new(format!( + "scope '{}' has leading/trailing whitespace", + scope + ))); + } + if scope.is_empty() { + return Err(ExecError::new("scope id cannot be empty")); + } + if !scope.contains(':') { + return Err(ExecError::new(format!( + "scope '{}' must contain ':' delimiter", + scope + ))); + } + if scope.starts_with(':') || scope.ends_with(':') || scope.contains("::") { + return Err(ExecError::new(format!( + "scope '{}' has invalid ':' placement", + scope + ))); + } + if !scope.chars().all(|c| { + c.is_ascii_lowercase() || c.is_ascii_digit() || c == ':' || c == '_' || c == '-' || c == '.' + }) { + return Err(ExecError::new(format!( + "scope '{}' contains invalid characters (allowed: [a-z0-9:_-.])", + scope + ))); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn map_to_gcp_inputs_allows_empty_service_account() { + let mut inputs = HashMap::new(); + inputs.insert("provider".to_string(), Value::Str("gcp".to_string())); + inputs.insert("runtime".to_string(), Value::Str("local".to_string())); + inputs.insert("audience".to_string(), Value::Str("local-dev".to_string())); + inputs.insert( + "project_or_account".to_string(), + Value::Str("gunb-secrets".to_string()), + ); + inputs.insert("secret".to_string(), Value::Str("dev-github".to_string())); + inputs.insert("scheme".to_string(), Value::Str("bearer".to_string())); + inputs.insert("source_id".to_string(), Value::Str("github".to_string())); + + let out = CloudOps::MapToGcpInputs { + runtime: CloudRuntimeKind::LocalDev, + } + .execute(inputs) + .expect("missing SA should not fail mapping"); + + assert_eq!(out.get("service_account").and_then(Value::as_str), Some("")); + } + + #[test] + fn map_to_gcp_inputs_prefers_impersonate_account() { + let mut inputs = HashMap::new(); + inputs.insert("provider".to_string(), Value::Str("gcp".to_string())); + inputs.insert("runtime".to_string(), Value::Str("local".to_string())); + inputs.insert("audience".to_string(), Value::Str("local-dev".to_string())); + inputs.insert( + "project_or_account".to_string(), + Value::Str("gunb-secrets".to_string()), + ); + inputs.insert("secret".to_string(), Value::Str("dev-github".to_string())); + inputs.insert("scheme".to_string(), Value::Str("bearer".to_string())); + inputs.insert("source_id".to_string(), Value::Str("github".to_string())); + inputs.insert( + "service_account_or_role".to_string(), + Value::Str("base@p.iam.gserviceaccount.com".to_string()), + ); + inputs.insert( + "impersonate_account_or_role".to_string(), + Value::Str("imp@p.iam.gserviceaccount.com".to_string()), + ); + + let out = CloudOps::MapToGcpInputs { + runtime: CloudRuntimeKind::LocalDev, + } + .execute(inputs) + .expect("mapping should succeed"); + + assert_eq!( + out.get("service_account").and_then(Value::as_str), + Some("imp@p.iam.gserviceaccount.com") + ); + } + + #[test] + fn map_to_gcp_inputs_passes_required_scopes() { + let mut inputs = HashMap::new(); + inputs.insert("provider".to_string(), Value::Str("gcp".to_string())); + inputs.insert("runtime".to_string(), Value::Str("local".to_string())); + inputs.insert("audience".to_string(), Value::Str("local-dev".to_string())); + inputs.insert( + "project_or_account".to_string(), + Value::Str("gunb-secrets".to_string()), + ); + inputs.insert("secret".to_string(), Value::Str("dev-github".to_string())); + inputs.insert("scheme".to_string(), Value::Str("bearer".to_string())); + inputs.insert("source_id".to_string(), Value::Str("github".to_string())); + inputs.insert( + "required_scopes".to_string(), + Value::str_list(vec![ + "llm:chat_completion".to_string(), + "review:code_review".to_string(), + ]), + ); + + let out = CloudOps::MapToGcpInputs { + runtime: CloudRuntimeKind::LocalDev, + } + .execute(inputs) + .expect("mapping should pass required scopes through"); + + assert_eq!( + out.get("required_scopes").and_then(Value::as_str_list), + Some(vec![ + "llm:chat_completion".to_string(), + "review:code_review".to_string(), + ]) + ); + } + + #[test] + fn scope_preflight_accepts_valid_scopes() { + let mut inputs = HashMap::new(); + inputs.insert( + "required_scopes".to_string(), + Value::str_list(vec![ + "gist:write".to_string(), + "review:code_review".to_string(), + ]), + ); + + let out = CloudOps::ScopePreflight + .execute(inputs) + .expect("valid required_scopes should pass"); + assert_eq!(out.get("scope_verified"), Some(&Value::Bool(true))); + } + + #[test] + fn scope_preflight_accepts_missing_required_scopes() { + let mut inputs = HashMap::new(); + inputs.insert("skip".to_string(), Value::Bool(false)); + + let out = CloudOps::ScopePreflight + .execute(inputs) + .expect("missing required_scopes should default to empty list"); + assert_eq!(out.get("scope_verified"), Some(&Value::Bool(true))); + } + + #[test] + fn scope_preflight_rejects_invalid_scope_id() { + let mut inputs = HashMap::new(); + inputs.insert( + "required_scopes".to_string(), + Value::str_list(vec!["BAD_SCOPE".to_string()]), + ); + + let err = CloudOps::ScopePreflight + .execute(inputs) + .expect_err("invalid scope format should fail"); + assert!( + err.to_string().contains("must contain ':' delimiter"), + "error should describe invalid scope format, got: {}", + err + ); + } +} diff --git a/lib/gcp-ops/src/graph.rs b/lib/gcp-ops/src/graph.rs index b96fae7c46b..e1ef601cec4 100644 --- a/lib/gcp-ops/src/graph.rs +++ b/lib/gcp-ops/src/graph.rs @@ -2,7 +2,7 @@ use crate::ops::{GcpOps, GcpRuntimeKind}; use gunbc_exec::{ExecError, Executable}; -use gunbc_ir::build::{optional, port, resource, AccessMode}; +use gunbc_ir::build::{list, optional, port, resource, AccessMode}; use gunbc_ir::{Dag, DagBuilder, Edge, Node, Value}; use gunbc_lib_transport::TransportOps; use gunbc_primitives::NetEnv; @@ -243,7 +243,6 @@ pub fn build_gcp_secret_manager_credential_graph( GcpRuntimeKind::LocalDev => { // Use the canonical upsert sub-DAG for local auth // (check -> create[guarded] -> resolve) - builder .add_root_node(Node::subdag( @@ -258,6 +257,18 @@ pub fn build_gcp_secret_manager_credential_graph( // Service Account impersonation // --------------------------------------------------------------------- + let should_impersonate = builder + .add_node_after( + Node::opaque( + "should_impersonate", + vec![port("service_account", "String")], + vec![port("should", "Bool")], + GcpSecretManagerGraphOp::Gcp(GcpOps::ShouldImpersonate), + ), + &access_token_node, + ) + .expect("should_impersonate"); + let prepare_impersonate = builder .add_node_after( Node::opaque( @@ -266,11 +277,12 @@ pub fn build_gcp_secret_manager_credential_graph( port("access_token", "String"), port("service_account", "String"), optional("lifetime_seconds", "OptionalInt"), + optional("should_impersonate", "OptionalBool"), ], vec![port("request", "TransportRequest"), port("skip", "Bool")], GcpSecretManagerGraphOp::Gcp(GcpOps::PrepareImpersonate), ), - &access_token_node, + &should_impersonate, ) .expect("prepare_impersonate"); @@ -294,7 +306,10 @@ pub fn build_gcp_secret_manager_credential_graph( .add_node_after( Node::opaque( "parse_impersonate", - vec![port("response", "TransportResponse")], + vec![ + port("response", "TransportResponse"), + optional("base_access_token", "OptionalString"), + ], vec![port("access_token", "String")], GcpSecretManagerGraphOp::Gcp(GcpOps::ParseImpersonate), ), @@ -308,6 +323,12 @@ pub fn build_gcp_secret_manager_credential_graph( prepare_impersonate.in_port("access_token"), ) .expect("access_token_node.access_token -> prepare_impersonate.access_token"); + builder + .add_edge( + should_impersonate.out("should"), + prepare_impersonate.in_port("should_impersonate"), + ) + .expect("should_impersonate.should -> prepare_impersonate.should_impersonate"); builder .add_edge( prepare_impersonate.out("request"), @@ -329,6 +350,12 @@ pub fn build_gcp_secret_manager_credential_graph( parse_impersonate.in_port("response"), ) .expect("execute_impersonate.response -> parse_impersonate.response"); + builder + .add_edge( + access_token_node.out("access_token"), + parse_impersonate.in_port("base_access_token"), + ) + .expect("access_token_node.access_token -> parse_impersonate.base_access_token"); // --------------------------------------------------------------------- // Secret Manager access @@ -417,6 +444,7 @@ pub fn build_gcp_secret_manager_credential_graph( port("scheme", "String"), optional("header_name", "OptionalString"), port("source_id", "String"), + list("required_scopes", "String"), ], vec![port("credential", "Credential")], GcpSecretManagerGraphOp::Gcp(GcpOps::BuildCredential), @@ -666,7 +694,6 @@ pub fn build_gcp_secret_manager_upsert_graph( GcpRuntimeKind::LocalDev => { // Use the canonical upsert sub-DAG for local auth // (check -> create[guarded] -> resolve) - builder .add_root_node(Node::subdag( @@ -681,6 +708,18 @@ pub fn build_gcp_secret_manager_upsert_graph( // Service Account impersonation // --------------------------------------------------------------------- + let should_impersonate = builder + .add_node_after( + Node::opaque( + "should_impersonate", + vec![port("service_account", "String")], + vec![port("should", "Bool")], + GcpSecretManagerGraphOp::Gcp(GcpOps::ShouldImpersonate), + ), + &access_token_node, + ) + .expect("should_impersonate"); + let prepare_impersonate = builder .add_node_after( Node::opaque( @@ -689,11 +728,12 @@ pub fn build_gcp_secret_manager_upsert_graph( port("access_token", "String"), port("service_account", "String"), optional("lifetime_seconds", "OptionalInt"), + optional("should_impersonate", "OptionalBool"), ], vec![port("request", "TransportRequest"), port("skip", "Bool")], GcpSecretManagerGraphOp::Gcp(GcpOps::PrepareImpersonate), ), - &access_token_node, + &should_impersonate, ) .expect("prepare_impersonate"); @@ -717,7 +757,10 @@ pub fn build_gcp_secret_manager_upsert_graph( .add_node_after( Node::opaque( "parse_impersonate", - vec![port("response", "TransportResponse")], + vec![ + port("response", "TransportResponse"), + optional("base_access_token", "OptionalString"), + ], vec![port("access_token", "String")], GcpSecretManagerGraphOp::Gcp(GcpOps::ParseImpersonate), ), @@ -731,6 +774,12 @@ pub fn build_gcp_secret_manager_upsert_graph( prepare_impersonate.in_port("access_token"), ) .expect("access_token_node.access_token -> prepare_impersonate.access_token"); + builder + .add_edge( + should_impersonate.out("should"), + prepare_impersonate.in_port("should_impersonate"), + ) + .expect("should_impersonate.should -> prepare_impersonate.should_impersonate"); builder .add_edge( prepare_impersonate.out("request"), @@ -752,6 +801,12 @@ pub fn build_gcp_secret_manager_upsert_graph( parse_impersonate.in_port("response"), ) .expect("execute_impersonate.response -> parse_impersonate.response"); + builder + .add_edge( + access_token_node.out("access_token"), + parse_impersonate.in_port("base_access_token"), + ) + .expect("access_token_node.access_token -> parse_impersonate.base_access_token"); // --------------------------------------------------------------------- // Secret Manager upsert: check -> create -> addVersion @@ -1038,10 +1093,20 @@ fn build_local_auth_upsert_dag() -> Dag { )); // Check edges - dag.add_edge(Edge::new("prepare_check", "request", "execute_check", "request")); + dag.add_edge(Edge::new( + "prepare_check", + "request", + "execute_check", + "request", + )); dag.add_edge(Edge::new("prepare_check", "skip", "execute_check", "skip")); dag.add_edge(Edge::new("net_env", "net", "execute_check", "res:net")); - dag.add_edge(Edge::new("execute_check", "response", "parse_check", "response")); + dag.add_edge(Edge::new( + "execute_check", + "response", + "parse_check", + "response", + )); // ======================================================================== // Resolve phase: read ADC -> parse credentials -> OAuth2 refresh -> parse token @@ -1115,20 +1180,70 @@ fn build_local_auth_upsert_dag() -> Dag { )); // Resolve edges: check -> read_adc -> parse_adc -> oauth2 -> parse_resolve - dag.add_edge(Edge::new("parse_check", "exists", "prepare_read_adc", "exists")); - dag.add_edge(Edge::new("prepare_read_adc", "request", "execute_read_adc", "request")); - dag.add_edge(Edge::new("prepare_read_adc", "skip", "execute_read_adc", "skip")); + dag.add_edge(Edge::new( + "parse_check", + "exists", + "prepare_read_adc", + "exists", + )); + dag.add_edge(Edge::new( + "prepare_read_adc", + "request", + "execute_read_adc", + "request", + )); + dag.add_edge(Edge::new( + "prepare_read_adc", + "skip", + "execute_read_adc", + "skip", + )); dag.add_edge(Edge::new("net_env", "net", "execute_read_adc", "res:net")); - dag.add_edge(Edge::new("execute_read_adc", "response", "parse_adc", "response")); + dag.add_edge(Edge::new( + "execute_read_adc", + "response", + "parse_adc", + "response", + )); - dag.add_edge(Edge::new("parse_adc", "client_id", "prepare_oauth2", "client_id")); - dag.add_edge(Edge::new("parse_adc", "client_secret", "prepare_oauth2", "client_secret")); - dag.add_edge(Edge::new("parse_adc", "refresh_token", "prepare_oauth2", "refresh_token")); + dag.add_edge(Edge::new( + "parse_adc", + "client_id", + "prepare_oauth2", + "client_id", + )); + dag.add_edge(Edge::new( + "parse_adc", + "client_secret", + "prepare_oauth2", + "client_secret", + )); + dag.add_edge(Edge::new( + "parse_adc", + "refresh_token", + "prepare_oauth2", + "refresh_token", + )); - dag.add_edge(Edge::new("prepare_oauth2", "request", "execute_oauth2", "request")); - dag.add_edge(Edge::new("prepare_oauth2", "skip", "execute_oauth2", "skip")); + dag.add_edge(Edge::new( + "prepare_oauth2", + "request", + "execute_oauth2", + "request", + )); + dag.add_edge(Edge::new( + "prepare_oauth2", + "skip", + "execute_oauth2", + "skip", + )); dag.add_edge(Edge::new("net_env", "net", "execute_oauth2", "res:net")); - dag.add_edge(Edge::new("execute_oauth2", "response", "parse_resolve", "response")); + dag.add_edge(Edge::new( + "execute_oauth2", + "response", + "parse_resolve", + "response", + )); dag } diff --git a/lib/gcp-ops/src/graph_mock.rs b/lib/gcp-ops/src/graph_mock.rs index 76caf318c90..4a9c32be1d5 100644 --- a/lib/gcp-ops/src/graph_mock.rs +++ b/lib/gcp-ops/src/graph_mock.rs @@ -74,6 +74,11 @@ pub fn gcp_github_mock_spec() -> MockSpec { "service_account", Value::Str("ci-secrets@mock.iam.gserviceaccount.com".into()), ) + .input_mock( + "parse_impersonate", + "response", + Value::Response(TransportResponse::Rest(impersonate_response.clone())), + ) .input_mock( "prepare_secret_access", "project", @@ -121,6 +126,7 @@ pub fn gcp_github_mock_spec() -> MockSpec { .skip_node_example("parse_github_oidc") .skip_node_example("prepare_sts") .skip_node_example("parse_sts") + .skip_node_example("should_impersonate") .skip_node_example("prepare_impersonate") .skip_node_example("parse_impersonate") .skip_node_example("prepare_secret_access") @@ -195,6 +201,11 @@ pub fn gcp_local_mock_spec() -> MockSpec { "service_account", Value::Str("ci-secrets@mock.iam.gserviceaccount.com".into()), ) + .input_mock( + "parse_impersonate", + "response", + Value::Response(TransportResponse::Rest(impersonate_response.clone())), + ) .input_mock( "prepare_secret_access", "project", @@ -229,6 +240,7 @@ pub fn gcp_local_mock_spec() -> MockSpec { ) // Sub-DAG internal nodes — tested at their own level .skip_node_example("local_auth_upsert") + .skip_node_example("should_impersonate") .skip_node_example("prepare_impersonate") .skip_node_example("parse_impersonate") .skip_node_example("prepare_secret_access") @@ -296,6 +308,11 @@ pub fn gcp_github_upsert_mock_spec() -> MockSpec { "service_account", Value::Str("ci-secrets@mock.iam.gserviceaccount.com".into()), ) + .input_mock( + "parse_impersonate", + "response", + Value::Response(TransportResponse::Rest(impersonate_response.clone())), + ) .input_mock( "prepare_secret_get", "project", @@ -364,6 +381,7 @@ pub fn gcp_github_upsert_mock_spec() -> MockSpec { .skip_node_example("parse_github_oidc") .skip_node_example("prepare_sts") .skip_node_example("parse_sts") + .skip_node_example("should_impersonate") .skip_node_example("prepare_impersonate") .skip_node_example("parse_impersonate") .skip_node_example("prepare_secret_get") diff --git a/lib/gcp-ops/src/ops.rs b/lib/gcp-ops/src/ops.rs index 7d8dfaf0e7e..cc2563caf7b 100644 --- a/lib/gcp-ops/src/ops.rs +++ b/lib/gcp-ops/src/ops.rs @@ -1,6 +1,9 @@ //! Pure GCP ops for WIF + Secret Manager. -use gunbc_exec::{require_bool, require_str, ExecError, Executable, OutputMap}; +use gunbc_exec::{ + optional_bool_strict, optional_str_list_strict, optional_str_strict, require_bool, require_str, + ExecError, Executable, OutputMap, +}; use gunbc_ir::transport::file::FileRequest; use gunbc_ir::transport::rest::RestRequest; use gunbc_ir::transport::TransportResponse; @@ -229,10 +232,10 @@ impl Executable for GcpOps { _ => false, }; if !exists { - return OutputMap::new() - .request("request", FileRequest::read("/dev/null").into()) - .bool("skip", true) - .ok(); + let path = adc_file_path(); + return Err(ExecError::new(format!( + "ADC file not found at {path}. Run `gcloud auth application-default login` and retry." + ))); } let adc_path = adc_file_path(); let req = FileRequest::read(&adc_path); @@ -273,9 +276,18 @@ impl Executable for GcpOps { .map_err(|e| ExecError::new(format!("ADC file is not valid JSON: {e}")))?; let client_id = json.get("client_id").and_then(|v| v.as_str()).unwrap_or(""); - let client_secret = json.get("client_secret").and_then(|v| v.as_str()).unwrap_or(""); - let refresh_token = json.get("refresh_token").and_then(|v| v.as_str()).unwrap_or(""); - let token_type = json.get("type").and_then(|v| v.as_str()).unwrap_or("authorized_user"); + let client_secret = json + .get("client_secret") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let refresh_token = json + .get("refresh_token") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let token_type = json + .get("type") + .and_then(|v| v.as_str()) + .unwrap_or("authorized_user"); if refresh_token.is_empty() { return Err(ExecError::new( @@ -311,8 +323,7 @@ impl Executable for GcpOps { "grant_type": "refresh_token", }); - let req = RestRequest::post("https://oauth2.googleapis.com/token") - .json(body); + let req = RestRequest::post("https://oauth2.googleapis.com/token").json(body); OutputMap::new() .request("request", req.into()) @@ -340,7 +351,9 @@ impl Executable for GcpOps { } }; if !rest.is_success() { - let error_desc = rest.body.get("error_description") + let error_desc = rest + .body + .get("error_description") .and_then(|v| v.as_str()) .unwrap_or("unknown error"); return Err(ExecError::new(format!( @@ -348,10 +361,16 @@ impl Executable for GcpOps { rest.status, error_desc ))); } - let access_token = rest.body.get("access_token") + let access_token = rest + .body + .get("access_token") .and_then(|v| v.as_str()) - .ok_or_else(|| ExecError::new("missing access_token in OAuth2 refresh response"))?; - let expires_in = rest.body.get("expires_in") + .ok_or_else(|| { + ExecError::new("missing access_token in OAuth2 refresh response") + })?; + let expires_in = rest + .body + .get("expires_in") .and_then(|v| v.as_i64()) .unwrap_or(3599); OutputMap::new() @@ -415,6 +434,19 @@ impl Executable for GcpOps { GcpOps::PrepareImpersonate => { let access_token = require_str(&inputs, "access_token")?; let service_account = require_str(&inputs, "service_account")?; + let should_impersonate = optional_bool_strict(&inputs, "should_impersonate")? + .unwrap_or_else(|| !service_account.trim().is_empty()); + if !should_impersonate || service_account.trim().is_empty() { + // No impersonation target configured; downstream parse node will + // fall back to the base access token. + return OutputMap::new() + .request( + "request", + RestRequest::post("https://example.invalid/impersonate").into(), + ) + .bool("skip", true) + .ok(); + } let lifetime_seconds = match inputs.get("lifetime_seconds") { Some(value) => value.as_int().ok_or_else(|| { ExecError::new("missing or invalid 'lifetime_seconds' input") @@ -441,8 +473,12 @@ impl Executable for GcpOps { .ok() } GcpOps::ParseImpersonate => { + let base_access_token = + optional_str_strict(&inputs, "base_access_token")?.unwrap_or(""); let response = match inputs.get("response") { - Some(Value::Skipped) => return OutputMap::new().str("access_token", "").ok(), + Some(Value::Skipped) => { + return OutputMap::new().str("access_token", base_access_token).ok() + } Some(Value::Response(r)) => r, _ => return Err(ExecError::new("missing or invalid 'response' input")), }; @@ -455,12 +491,24 @@ impl Executable for GcpOps { ))); } }; + if !rest.is_success() { + let details = impersonation_error_summary(&rest.body); + return Err(ExecError::new(format!( + "impersonation failed (status {}): {}", + rest.status, details + ))); + } let token = rest .body .get("accessToken") + .or_else(|| rest.body.get("access_token")) .and_then(|v| v.as_str()) .ok_or_else(|| { - ExecError::new("missing accessToken in impersonation response") + let details = impersonation_error_summary(&rest.body); + ExecError::new(format!( + "impersonation response missing accessToken (status {}): {}", + rest.status, details + )) })?; let _ = rest .body @@ -668,6 +716,8 @@ impl Executable for GcpOps { GcpOps::BuildCredential => { let secret = require_str(&inputs, "secret")?; let scheme = require_str(&inputs, "scheme")?; + let _required_scopes = + optional_str_list_strict(&inputs, "required_scopes")?.unwrap_or_default(); let header_name = match inputs.get("header_name") { Some(value) => Some(value.as_str().ok_or_else(|| { @@ -707,8 +757,11 @@ impl Executable for GcpOps { OutputMap::new().value("credential", cred.into()).ok() } GcpOps::ShouldImpersonate => { - let service_account = require_str(&inputs, "service_account")?; - let should = !service_account.trim().is_empty(); + let should = inputs + .get("service_account") + .and_then(Value::as_str) + .map(|s| !s.trim().is_empty()) + .unwrap_or(false); OutputMap::new().bool("should", should).ok() } GcpOps::ComposeSecretName => { @@ -859,6 +912,37 @@ fn base64_decode(input: &str) -> Result, String> { Ok(out) } +fn impersonation_error_summary(body: &serde_json::Value) -> String { + if let Some(err) = body.get("error") { + if let Some(message) = err.get("message").and_then(|v| v.as_str()) { + return message.to_string(); + } + if let Some(status) = err.get("status").and_then(|v| v.as_str()) { + return status.to_string(); + } + if let Some(code) = err.get("code").and_then(|v| v.as_i64()) { + return format!("error code {code}"); + } + } + + if let Some(message) = body.get("message").and_then(|v| v.as_str()) { + return message.to_string(); + } + if let Some(desc) = body.get("error_description").and_then(|v| v.as_str()) { + return desc.to_string(); + } + if let Some(status) = body.get("status").and_then(|v| v.as_str()) { + return status.to_string(); + } + + let rendered = body.to_string(); + if rendered.len() > 240 { + format!("{}...", &rendered[..240]) + } else { + rendered + } +} + #[cfg(test)] mod tests { use super::*; @@ -872,7 +956,9 @@ mod tests { #[test] fn prepare_check_adc_produces_file_exists_request() { let inputs = HashMap::new(); - let outputs = GcpOps::PrepareCheckAdc.execute(inputs).expect("should succeed"); + let outputs = GcpOps::PrepareCheckAdc + .execute(inputs) + .expect("should succeed"); let req = outputs.get("request").expect("should have request"); // The request should be a File transport request match req { @@ -889,11 +975,14 @@ mod tests { let mut inputs = HashMap::new(); inputs.insert( "response".to_string(), - Value::Response(TransportResponse::File( - FileResponse::exists_result("/fake/path", false), - )), + Value::Response(TransportResponse::File(FileResponse::exists_result( + "/fake/path", + false, + ))), ); - let outputs = GcpOps::ParseCheckAdc.execute(inputs).expect("should succeed"); + let outputs = GcpOps::ParseCheckAdc + .execute(inputs) + .expect("should succeed"); assert_eq!(outputs.get("exists"), Some(&Value::Bool(false))); } @@ -902,11 +991,14 @@ mod tests { let mut inputs = HashMap::new(); inputs.insert( "response".to_string(), - Value::Response(TransportResponse::File( - FileResponse::exists_result("/fake/path", true), - )), + Value::Response(TransportResponse::File(FileResponse::exists_result( + "/fake/path", + true, + ))), ); - let outputs = GcpOps::ParseCheckAdc.execute(inputs).expect("should succeed"); + let outputs = GcpOps::ParseCheckAdc + .execute(inputs) + .expect("should succeed"); assert_eq!(outputs.get("exists"), Some(&Value::Bool(true))); } @@ -914,7 +1006,9 @@ mod tests { fn prepare_read_adc_produces_file_read_request() { let mut inputs = HashMap::new(); inputs.insert("exists".to_string(), Value::Bool(true)); - let outputs = GcpOps::PrepareReadAdc.execute(inputs).expect("should succeed"); + let outputs = GcpOps::PrepareReadAdc + .execute(inputs) + .expect("should succeed"); match outputs.get("request") { Some(Value::Request(gunbc_ir::transport::TransportRequest::File(f))) => { assert!(f.path.contains("application_default_credentials.json")); @@ -925,11 +1019,18 @@ mod tests { } #[test] - fn prepare_read_adc_skips_when_not_exists() { + fn prepare_read_adc_fails_when_not_exists() { let mut inputs = HashMap::new(); inputs.insert("exists".to_string(), Value::Bool(false)); - let outputs = GcpOps::PrepareReadAdc.execute(inputs).expect("should succeed"); - assert_eq!(outputs.get("skip"), Some(&Value::Bool(true))); + let err = GcpOps::PrepareReadAdc + .execute(inputs) + .expect_err("missing ADC should fail early"); + assert!( + err.to_string() + .contains("gcloud auth application-default login"), + "error should include remediation command, got: {}", + err + ); } #[test] @@ -943,11 +1044,14 @@ mod tests { let mut inputs = HashMap::new(); inputs.insert( "response".to_string(), - Value::Response(TransportResponse::File( - FileResponse::read_ok("/fake/path", adc_json.to_string()), - )), + Value::Response(TransportResponse::File(FileResponse::read_ok( + "/fake/path", + adc_json.to_string(), + ))), ); - let outputs = GcpOps::ParseAdcCredentials.execute(inputs).expect("should succeed"); + let outputs = GcpOps::ParseAdcCredentials + .execute(inputs) + .expect("should succeed"); assert_eq!( outputs.get("client_id").and_then(|v| v.as_str()), Some("my-client-id.apps.googleusercontent.com") @@ -972,9 +1076,10 @@ mod tests { let mut inputs = HashMap::new(); inputs.insert( "response".to_string(), - Value::Response(TransportResponse::File( - FileResponse::read_ok("/fake/path", adc_json.to_string()), - )), + Value::Response(TransportResponse::File(FileResponse::read_ok( + "/fake/path", + adc_json.to_string(), + ))), ); let err = GcpOps::ParseAdcCredentials .execute(inputs) @@ -986,9 +1091,14 @@ mod tests { fn prepare_oauth2_refresh_builds_rest_request() { let mut inputs = HashMap::new(); inputs.insert("client_id".to_string(), Value::Str("id".to_string())); - inputs.insert("client_secret".to_string(), Value::Str("secret".to_string())); + inputs.insert( + "client_secret".to_string(), + Value::Str("secret".to_string()), + ); inputs.insert("refresh_token".to_string(), Value::Str("token".to_string())); - let outputs = GcpOps::PrepareOAuth2Refresh.execute(inputs).expect("should succeed"); + let outputs = GcpOps::PrepareOAuth2Refresh + .execute(inputs) + .expect("should succeed"); match outputs.get("request") { Some(Value::Request(gunbc_ir::transport::TransportRequest::Rest(r))) => { assert!(r.url.contains("oauth2.googleapis.com/token")); @@ -1013,12 +1123,17 @@ mod tests { }), ))), ); - let outputs = GcpOps::ParseOAuth2Refresh.execute(inputs).expect("should succeed"); + let outputs = GcpOps::ParseOAuth2Refresh + .execute(inputs) + .expect("should succeed"); assert_eq!( outputs.get("access_token").and_then(|v| v.as_str()), Some("ya29.a-token") ); - assert_eq!(outputs.get("expires_in").and_then(|v| v.as_int()), Some(3600)); + assert_eq!( + outputs.get("expires_in").and_then(|v| v.as_int()), + Some(3600) + ); } #[test] @@ -1039,4 +1154,124 @@ mod tests { .expect_err("should fail on error response"); assert!(err.to_string().contains("expired or revoked")); } + + #[test] + fn parse_impersonate_reports_status_and_error_message() { + let mut inputs = HashMap::new(); + inputs.insert( + "response".to_string(), + Value::Response(TransportResponse::Rest(RestResponse::new( + 403, + serde_json::json!({ + "error": { + "code": 403, + "message": "Permission iam.serviceAccounts.getAccessToken denied", + "status": "PERMISSION_DENIED" + } + }), + ))), + ); + + let err = GcpOps::ParseImpersonate + .execute(inputs) + .expect_err("impersonation failure should bubble up"); + let msg = err.to_string(); + assert!(msg.contains("status 403"), "msg: {msg}"); + assert!( + msg.contains("Permission iam.serviceAccounts.getAccessToken denied"), + "msg: {msg}" + ); + } + + #[test] + fn parse_impersonate_accepts_access_token_snake_case() { + let mut inputs = HashMap::new(); + inputs.insert( + "response".to_string(), + Value::Response(TransportResponse::Rest(RestResponse::ok( + serde_json::json!({ + "access_token": "ya29.impersonated" + }), + ))), + ); + + let outputs = GcpOps::ParseImpersonate + .execute(inputs) + .expect("snake_case access token should parse"); + assert_eq!( + outputs.get("access_token").and_then(|v| v.as_str()), + Some("ya29.impersonated") + ); + } + + #[test] + fn prepare_impersonate_skips_when_service_account_empty() { + let mut inputs = HashMap::new(); + inputs.insert( + "access_token".to_string(), + Value::Str("base-token".to_string()), + ); + inputs.insert("service_account".to_string(), Value::Str(String::new())); + + let outputs = GcpOps::PrepareImpersonate + .execute(inputs) + .expect("empty service account should skip impersonation"); + assert_eq!(outputs.get("skip"), Some(&Value::Bool(true))); + } + + #[test] + fn parse_impersonate_uses_base_token_when_skipped() { + let mut inputs = HashMap::new(); + inputs.insert("response".to_string(), Value::Skipped); + inputs.insert( + "base_access_token".to_string(), + Value::Str("base-token".to_string()), + ); + + let outputs = GcpOps::ParseImpersonate + .execute(inputs) + .expect("skipped impersonation should return base token"); + assert_eq!( + outputs.get("access_token").and_then(|v| v.as_str()), + Some("base-token") + ); + } + + #[test] + fn build_credential_accepts_required_scopes_list() { + let mut inputs = HashMap::new(); + inputs.insert("secret".to_string(), Value::Str("tok".to_string())); + inputs.insert("scheme".to_string(), Value::Str("bearer".to_string())); + inputs.insert("source_id".to_string(), Value::Str("openai".to_string())); + inputs.insert( + "required_scopes".to_string(), + Value::str_list(vec!["llm:chat_completion".to_string()]), + ); + + let outputs = GcpOps::BuildCredential + .execute(inputs) + .expect("valid required_scopes list should be accepted"); + assert!( + outputs.contains_key("credential"), + "build_credential should return a credential" + ); + } + + #[test] + fn build_credential_rejects_wrong_required_scopes_type() { + let mut inputs = HashMap::new(); + inputs.insert("secret".to_string(), Value::Str("tok".to_string())); + inputs.insert("scheme".to_string(), Value::Str("bearer".to_string())); + inputs.insert("source_id".to_string(), Value::Str("openai".to_string())); + inputs.insert("required_scopes".to_string(), Value::Int(1)); + + let err = GcpOps::BuildCredential + .execute(inputs) + .expect_err("wrong required_scopes type should fail"); + assert!( + err.to_string().contains("required_scopes"), + "error should mention required_scopes, got: {}", + err + ); + } } diff --git a/lib/llm-ops/src/graph.rs b/lib/llm-ops/src/graph.rs index f09439d78bc..7c73d933833 100644 --- a/lib/llm-ops/src/graph.rs +++ b/lib/llm-ops/src/graph.rs @@ -3,19 +3,21 @@ //! Provides a composable DAG for the LLM chat completion pattern: //! //! ```text -//! PrepareChatRequest (pure) → ResolveAuth (pure) → ConstCloudConfig → CloudSecretManager (subdag) → TransportOps::Execute (I/O) → ParseChatResponse (pure) +//! PrepareChatRequest (pure) → ResolveAuth (pure) → ScopePreflight (pure) +//! → ConstCloudConfig → CloudSecretManager (subdag) +//! → TransportOps::Execute (I/O) → ParseChatResponse (pure) //! ``` //! //! This graph can be embedded as a sub-DAG in larger workflows that need //! LLM capabilities (code review, code generation, etc.). use gunbc_exec::{ExecError, Executable}; +use gunbc_ir::transport::cloud::CloudSecretConfig; use gunbc_ir::{ add_transport_execute_parse_named_with_passthrough, build::*, Dag, DagBuilder, Node, Value, }; -use gunbc_ir::transport::cloud::CloudSecretConfig; use gunbc_lib_cloud_ops::{ - build_cloud_secret_manager_credential_graph_from_config, default_local_dev_config, CloudOps, + build_cloud_secret_manager_credential_graph_from_config, graph_cloud_config, CloudOps, CloudSecretManagerGraphOp, }; use gunbc_lib_transport::TransportOps; @@ -63,13 +65,11 @@ impl Executable for LlmGraphOp { /// - `parse.input_tokens`: Int /// - `parse.output_tokens`: Int pub fn build_chat_completion_graph() -> Dag { - build_chat_completion_graph_with_config(default_local_dev_config()) + build_chat_completion_graph_with_config(graph_cloud_config()) } /// Build a chat completion DAG with explicit cloud config. -pub fn build_chat_completion_graph_with_config( - cloud_config: CloudSecretConfig, -) -> Dag { +pub fn build_chat_completion_graph_with_config(cloud_config: CloudSecretConfig) -> Dag { let mut builder: DagBuilder = DagBuilder::new(); // Node 0: Cloud environment (config + OIDC request inputs) @@ -121,6 +121,7 @@ pub fn build_chat_completion_graph_with_config( port("service", "String"), port("scheme", "String"), port("header_name", "String"), + list("required_scopes", "String"), port("interactive_allowed", "Bool"), ], LlmGraphOp::Llm(LlmOps::ResolveAuth), @@ -147,20 +148,37 @@ pub fn build_chat_completion_graph_with_config( .expect("bind_secret node"); // Node 4: Cloud credential acquisition graph (GCP WIF + Secret Manager) - let cloud_subdag = - lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config(&cloud_config)); + let cloud_subdag = lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config( + &cloud_config, + )); let cloud_credential = builder .add_node_after(Node::subdag("cloud_credential", cloud_subdag), &bind_secret) .expect("cloud_credential node"); - // Nodes 5-6: Execute transport + ParseChatResponse + // Node 5: Scope preflight gate (pure; fails fast on invalid/empty scopes) + let scope_preflight = builder + .add_node_after( + Node::opaque( + "scope_preflight", + vec![list("required_scopes", "String")], + vec![port("scope_verified", "Bool")], + LlmGraphOp::Cloud(CloudSecretManagerGraphOp::Cloud(CloudOps::ScopePreflight)), + ), + &resolve_auth, + ) + .expect("scope_preflight node"); + + // Nodes 6-7: Execute transport + ParseChatResponse let llm_triplet = add_transport_execute_parse_named_with_passthrough( &mut builder, &prepare, "execute", "parse", vec![port("provider", "String")], - vec![resource("credential", "Credential", AccessMode::Read)], + vec![ + optional("scope_verified", "OptionalBool"), + resource("credential", "Credential", AccessMode::Read), + ], vec![ port("content", "String"), port("model", "String"), @@ -178,6 +196,18 @@ pub fn build_chat_completion_graph_with_config( builder .add_edge(prepare.out("provider"), resolve_auth.in_port("provider")) .expect("prepare.provider -> resolve_auth.provider"); + builder + .add_edge( + resolve_auth.out("required_scopes"), + scope_preflight.in_port("required_scopes"), + ) + .expect("resolve_auth.required_scopes -> scope_preflight.required_scopes"); + builder + .add_edge( + scope_preflight.out("scope_verified"), + llm_triplet.execute.in_port("scope_verified"), + ) + .expect("scope_preflight.scope_verified -> execute.scope_verified"); builder .add_edge(cloud_env.out("config"), bind_secret.in_port("config")) .expect("cloud_env.config -> bind_secret.config"); @@ -214,6 +244,12 @@ pub fn build_chat_completion_graph_with_config( cloud_credential.in_port("interactive_allowed"), ) .expect("resolve_auth.interactive_allowed -> cloud_credential.interactive_allowed"); + builder + .add_edge( + resolve_auth.out("required_scopes"), + cloud_credential.in_port("required_scopes"), + ) + .expect("resolve_auth.required_scopes -> cloud_credential.required_scopes"); builder .add_edge( cloud_env.out("request_url"), diff --git a/lib/llm-ops/src/graph_mock.rs b/lib/llm-ops/src/graph_mock.rs index ecb114bb3ec..2ef8223f7d3 100644 --- a/lib/llm-ops/src/graph_mock.rs +++ b/lib/llm-ops/src/graph_mock.rs @@ -201,6 +201,7 @@ pub fn openai_mock_spec() -> MockSpec { .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } /// Mock specification for Anthropic chat completion. @@ -340,6 +341,7 @@ pub fn anthropic_mock_spec() -> MockSpec { .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } /// Mock specification for code review workflow. @@ -448,6 +450,7 @@ Overall: The code is clean and well-structured. Minor fixes recommended."; .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } /// Mock specification for testing API key as secret. @@ -554,6 +557,7 @@ pub fn secret_api_key_mock_spec() -> MockSpec { .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } /// Mock specification for testing rate limiting / error scenarios. @@ -607,6 +611,7 @@ pub fn rate_limited_mock_spec() -> MockSpec { .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } /// Mock specification for credential lifecycle testing. @@ -734,6 +739,7 @@ pub fn credential_lifecycle_mock_spec() -> MockSpec { .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } /// Mock specification for Anthropic credential lifecycle testing. @@ -875,4 +881,5 @@ pub fn credential_lifecycle_anthropic_mock_spec() -> MockSpec { .skip_node_example("cloud_env") .skip_node_example("cloud_credential") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } diff --git a/lib/llm-ops/src/lib.rs b/lib/llm-ops/src/lib.rs index cee3bc5aec1..465e75ef8f4 100644 --- a/lib/llm-ops/src/lib.rs +++ b/lib/llm-ops/src/lib.rs @@ -31,7 +31,7 @@ use gunbc_exec::{ TransportResponseExt, }; use gunbc_ir::transport::llm::{self, ChatMessage, ChatRequest, MessageContent, Role}; -use gunbc_ir::transport::TransportRequest; +use gunbc_ir::transport::{ScopeContract, TransportRequest}; use gunbc_ir::Value; use std::collections::HashMap; @@ -47,9 +47,10 @@ pub enum LlmOps { /// /// Outputs: /// - `service`: String - canonical provider/service ID - /// - `env_var`: String - environment variable for the API key /// - `scheme`: String - auth scheme ("bearer" or "header") /// - `header_name`: String - header name for "header" scheme (e.g., "x-api-key"), empty for "bearer" + /// - `required_scopes`: List - required capability scopes for this request class + /// - `interactive_allowed`: Bool - whether interactive recovery is allowed ResolveAuth, /// Build a chat completion REST request from inputs. /// @@ -140,11 +141,20 @@ fn execute_resolve_auth( )) } }; + let provider_id = provider.id.clone(); + let intent = llm::LlmScopeContract::new(provider_id.clone()).credential_intent(); + intent.validate().map_err(|e| { + ExecError::new(format!( + "invalid llm credential contract for '{}': {e}", + provider_id + )) + })?; OutputMap::new() - .str("service", provider.id) + .str("service", provider_id) .str("scheme", scheme) .str("header_name", header_name) + .str_list("required_scopes", intent.required_scopes) .bool("interactive_allowed", true) .ok() } @@ -577,6 +587,10 @@ mod tests { Some(&Value::Str("bearer".to_string())) ); assert_eq!(result.get("header_name"), Some(&Value::Str(String::new()))); + assert_eq!( + result.get("required_scopes"), + Some(&Value::str_list(vec!["llm:chat_completion".to_string()])) + ); assert_eq!(result.get("interactive_allowed"), Some(&Value::Bool(true))); } @@ -594,6 +608,10 @@ mod tests { result.get("header_name"), Some(&Value::Str("x-api-key".to_string())) ); + assert_eq!( + result.get("required_scopes"), + Some(&Value::str_list(vec!["llm:chat_completion".to_string()])) + ); } #[test] diff --git a/lib/review/src/graph.rs b/lib/review/src/graph.rs index e7a0ef6f34d..5952904caaf 100644 --- a/lib/review/src/graph.rs +++ b/lib/review/src/graph.rs @@ -11,14 +11,14 @@ //! 2. LLM call use gunbc_exec::{ExecError, Executable}; +use gunbc_ir::transport::cloud::CloudSecretConfig; use gunbc_ir::{ add_transport_execute_parse_named_with_passthrough, add_transport_triplet_named_with_passthrough, build::*, Dag, DagBuilder, Node, NodeRef, Value, }; use gunbc_lib_blob::BlobOps; -use gunbc_ir::transport::cloud::CloudSecretConfig; use gunbc_lib_cloud_ops::{ - build_cloud_secret_manager_credential_graph_from_config, default_local_dev_config, CloudOps, + build_cloud_secret_manager_credential_graph_from_config, graph_cloud_config, CloudOps, CloudSecretManagerGraphOp, }; use gunbc_lib_git_ops::GitOps; @@ -101,8 +101,9 @@ fn add_cloud_credential_chain( .add_edge(resolve_auth.out("service"), bind_secret.in_port("service")) .expect("resolve_auth.service -> bind_secret.service"); - let cloud_subdag = - lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config(cloud_config)); + let cloud_subdag = lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config( + cloud_config, + )); let cloud_credential = builder .add_node_after(Node::subdag("cloud_credential", cloud_subdag), &bind_secret) .expect("cloud_credential node"); @@ -137,6 +138,12 @@ fn add_cloud_credential_chain( cloud_credential.in_port("interactive_allowed"), ) .expect("resolve_auth.interactive_allowed -> cloud_credential.interactive_allowed"); + builder + .add_edge( + resolve_auth.out("required_scopes"), + cloud_credential.in_port("required_scopes"), + ) + .expect("resolve_auth.required_scopes -> cloud_credential.required_scopes"); builder .add_edge( cloud_env.out("request_url"), @@ -153,6 +160,32 @@ fn add_cloud_credential_chain( cloud_credential } +fn add_scope_preflight_chain( + builder: &mut DagBuilder, + resolve_auth: &NodeRef, +) -> NodeRef { + let scope_preflight = builder + .add_node_after( + Node::opaque( + "scope_preflight", + vec![list("required_scopes", "String")], + vec![port("scope_verified", "Bool")], + ReviewGraphOp::Cloud(CloudSecretManagerGraphOp::Cloud(CloudOps::ScopePreflight)), + ), + resolve_auth, + ) + .expect("scope_preflight node"); + + builder + .add_edge( + resolve_auth.out("required_scopes"), + scope_preflight.in_port("required_scopes"), + ) + .expect("resolve_auth.required_scopes -> scope_preflight.required_scopes"); + + scope_preflight +} + fn lift_cloud_dag(dag: Dag) -> Dag { dag.map_ops(&mut ReviewGraphOp::Cloud) } @@ -217,13 +250,11 @@ fn add_cloud_env_node( builder = "build_review_phase_graph()" )] pub fn build_review_phase_graph() -> Dag { - build_review_phase_graph_with_config(default_local_dev_config()) + build_review_phase_graph_with_config(graph_cloud_config()) } /// Build a ReviewPhase DAG with an explicit cloud config. -pub fn build_review_phase_graph_with_config( - cloud_config: CloudSecretConfig, -) -> Dag { +pub fn build_review_phase_graph_with_config(cloud_config: CloudSecretConfig) -> Dag { let mut builder: DagBuilder = DagBuilder::new(); let fs_env = builder @@ -349,9 +380,10 @@ pub fn build_review_phase_graph_with_config( port("service", "String"), port("scheme", "String"), port("header_name", "String"), + list("required_scopes", "String"), port("interactive_allowed", "Bool"), ], - ReviewGraphOp::Llm(LlmOps::ResolveAuth), + ReviewGraphOp::Review(ReviewOps::ResolveAuthContract), ), &prepare_llm, ) @@ -360,21 +392,31 @@ pub fn build_review_phase_graph_with_config( // Node 7: Cloud credential acquisition (resolves provider credentials) let cloud_credential = add_cloud_credential_chain(&mut builder, &cloud_env, &resolve_auth, &cloud_config); + let scope_preflight = add_scope_preflight_chain(&mut builder, &resolve_auth); - // Nodes 8-9: Execute LLM + ParseSimpleResponse + // Nodes 8-10: ScopePreflight -> Execute LLM + ParseSimpleResponse let llm_triplet = add_transport_execute_parse_named_with_passthrough( &mut builder, &prepare_llm, "execute_llm", "parse_llm", vec![port("provider", "String")], - vec![resource("credential", "Credential", AccessMode::Read)], + vec![ + optional("scope_verified", "OptionalBool"), + resource("credential", "Credential", AccessMode::Read), + ], vec![port("answer", "String")], ReviewGraphOp::Llm(LlmOps::ParseSimpleResponse), ReviewGraphOp::Transport(TransportOps::Execute), Some(&cloud_credential), ) .expect("llm triplet"); + builder + .add_edge( + scope_preflight.out("scope_verified"), + llm_triplet.execute.in_port("scope_verified"), + ) + .expect("scope_preflight.scope_verified -> execute_llm.scope_verified"); // ======================================================================== // Review Response Parsing @@ -472,7 +514,7 @@ pub fn build_review_phase_graph_with_config( /// - `parse_response.output`: Json — ReviewOutput /// - `parse_response.errors`: Json — Parse errors array pub fn build_inline_review_graph() -> Dag { - build_inline_review_graph_with_config(default_local_dev_config()) + build_inline_review_graph_with_config(graph_cloud_config()) } /// Build an inline review graph with explicit cloud config. @@ -531,9 +573,10 @@ pub fn build_inline_review_graph_with_config( port("service", "String"), port("scheme", "String"), port("header_name", "String"), + list("required_scopes", "String"), port("interactive_allowed", "Bool"), ], - ReviewGraphOp::Llm(LlmOps::ResolveAuth), + ReviewGraphOp::Review(ReviewOps::ResolveAuthContract), ), &prepare_llm, ) @@ -542,6 +585,7 @@ pub fn build_inline_review_graph_with_config( // Node 4: Cloud credential acquisition (resolves provider credentials) let cloud_credential = add_cloud_credential_chain(&mut builder, &cloud_env, &resolve_auth, &cloud_config); + let scope_preflight = add_scope_preflight_chain(&mut builder, &resolve_auth); // Nodes 5-6: Execute LLM + ParseSimpleResponse let llm_triplet = add_transport_execute_parse_named_with_passthrough( @@ -550,13 +594,22 @@ pub fn build_inline_review_graph_with_config( "execute_llm", "parse_llm", vec![port("provider", "String")], - vec![resource("credential", "Credential", AccessMode::Read)], + vec![ + optional("scope_verified", "OptionalBool"), + resource("credential", "Credential", AccessMode::Read), + ], vec![port("answer", "String")], ReviewGraphOp::Llm(LlmOps::ParseSimpleResponse), ReviewGraphOp::Transport(TransportOps::Execute), Some(&cloud_credential), ) .expect("llm triplet"); + builder + .add_edge( + scope_preflight.out("scope_verified"), + llm_triplet.execute.in_port("scope_verified"), + ) + .expect("scope_preflight.scope_verified -> execute_llm.scope_verified"); // Node 7: ParseReviewResponse let parse_response = builder @@ -653,7 +706,7 @@ pub fn build_diff_review_graph() -> Dag { /// - Two TransportOps::Execute calls: git diff (read), LLM (read) /// - Phase overall: Read-only pub fn build_diff_review_graph_with(config: ReviewPipelineConfig) -> Dag { - build_diff_review_graph_with_cloud_config(config, default_local_dev_config()) + build_diff_review_graph_with_cloud_config(config, graph_cloud_config()) } /// Build a DiffReviewPhase DAG with explicit cloud config. @@ -794,9 +847,10 @@ pub fn build_diff_review_graph_with_cloud_config( port("service", "String"), port("scheme", "String"), port("header_name", "String"), + list("required_scopes", "String"), port("interactive_allowed", "Bool"), ], - ReviewGraphOp::Llm(LlmOps::ResolveAuth), + ReviewGraphOp::Review(ReviewOps::ResolveAuthContract), ), &prepare_llm, ) @@ -804,6 +858,7 @@ pub fn build_diff_review_graph_with_cloud_config( let cloud_credential = add_cloud_credential_chain(&mut builder, &cloud_env, &resolve_auth, &cloud_config); + let scope_preflight = add_scope_preflight_chain(&mut builder, &resolve_auth); let llm_triplet = add_transport_execute_parse_named_with_passthrough( &mut builder, @@ -811,13 +866,22 @@ pub fn build_diff_review_graph_with_cloud_config( "execute_llm", "parse_llm", vec![port("provider", "String")], - vec![resource("credential", "Credential", AccessMode::Read)], + vec![ + optional("scope_verified", "OptionalBool"), + resource("credential", "Credential", AccessMode::Read), + ], vec![port("answer", "String")], ReviewGraphOp::Llm(LlmOps::ParseSimpleResponse), ReviewGraphOp::Transport(TransportOps::Execute), Some(&cloud_credential), ) .expect("llm triplet"); + builder + .add_edge( + scope_preflight.out("scope_verified"), + llm_triplet.execute.in_port("scope_verified"), + ) + .expect("scope_preflight.scope_verified -> execute_llm.scope_verified"); // ======================================================================== // Review Response Parsing @@ -965,7 +1029,7 @@ pub fn build_multi_source_review_graph() -> Dag { /// Build a MultiSourceReviewPhase DAG with explicit pipeline config. pub fn build_multi_source_review_graph_with(config: ReviewPipelineConfig) -> Dag { - build_multi_source_review_graph_with_cloud_config(config, default_local_dev_config()) + build_multi_source_review_graph_with_cloud_config(config, graph_cloud_config()) } /// Build a MultiSourceReviewPhase DAG with explicit pipeline and cloud configs. @@ -1046,9 +1110,10 @@ pub fn build_multi_source_review_graph_with_cloud_config( port("service", "String"), port("scheme", "String"), port("header_name", "String"), + list("required_scopes", "String"), port("interactive_allowed", "Bool"), ], - ReviewGraphOp::Llm(LlmOps::ResolveAuth), + ReviewGraphOp::Review(ReviewOps::ResolveAuthContract), ), &prepare_llm, ) @@ -1056,6 +1121,7 @@ pub fn build_multi_source_review_graph_with_cloud_config( let cloud_credential = add_cloud_credential_chain(&mut builder, &cloud_env, &resolve_auth, &cloud_config); + let scope_preflight = add_scope_preflight_chain(&mut builder, &resolve_auth); let llm_triplet = add_transport_execute_parse_named_with_passthrough( &mut builder, @@ -1063,13 +1129,22 @@ pub fn build_multi_source_review_graph_with_cloud_config( "execute_llm", "parse_llm", vec![port("provider", "String")], - vec![resource("credential", "Credential", AccessMode::Read)], + vec![ + optional("scope_verified", "OptionalBool"), + resource("credential", "Credential", AccessMode::Read), + ], vec![port("answer", "String")], ReviewGraphOp::Llm(LlmOps::ParseSimpleResponse), ReviewGraphOp::Transport(TransportOps::Execute), Some(&cloud_credential), ) .expect("llm triplet"); + builder + .add_edge( + scope_preflight.out("scope_verified"), + llm_triplet.execute.in_port("scope_verified"), + ) + .expect("scope_preflight.scope_verified -> execute_llm.scope_verified"); let parse_response = builder .add_node_after( diff --git a/lib/review/src/graph_mock.rs b/lib/review/src/graph_mock.rs index 3d178bb06c2..47dab7dca5c 100644 --- a/lib/review/src/graph_mock.rs +++ b/lib/review/src/graph_mock.rs @@ -269,6 +269,7 @@ pub fn inline_review_mock_spec() -> MockSpec { .skip_node_example("cloud_credential") .skip_node_example("fs_env") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } // ============================================================================ @@ -468,4 +469,5 @@ diff --git a/src/main.rs b/src/main.rs .skip_node_example("cloud_credential") .skip_node_example("fs_env") .skip_node_example("bind_secret") + .skip_node_example("scope_preflight") } diff --git a/lib/review/src/lib.rs b/lib/review/src/lib.rs index 5e81d1f9e29..fb609b217c1 100644 --- a/lib/review/src/lib.rs +++ b/lib/review/src/lib.rs @@ -27,6 +27,8 @@ use gunbc_exec::{ optional_str_strict, propagate_skipped, require_json, require_map_str_str, require_str, ExecError, Executable, IntoExecResult, OutputMap, }; +use gunbc_ir::transport::review::ReviewScopeContract; +use gunbc_ir::transport::ScopeContract; use gunbc_ir::Value; use gunbc_primitives::{FormatMapOp, StableHashOp}; use serde::{Deserialize, Serialize}; @@ -251,6 +253,19 @@ pub enum ReviewOps { /// - `system_prompt`: String - system prompt for LLM PrepareReviewPrompt, + /// Resolve review auth requirements from the review scope contract. + /// + /// Inputs: + /// - `provider`: String - LLM provider ID ("openai", "anthropic") + /// + /// Outputs: + /// - `service`: String - credential service ID + /// - `scheme`: String - auth scheme ("bearer" or "header") + /// - `header_name`: String - header name for "header" scheme + /// - `required_scopes`: List - includes `review:code_review` + /// - `interactive_allowed`: Bool - whether interactive remediation is allowed + ResolveAuthContract, + /// Parse an LLM response into structured findings. /// /// Inputs: @@ -301,6 +316,7 @@ impl Executable for ReviewOps { match self { ReviewOps::LoadPipelineConfig(config) => execute_load_pipeline_config(config), ReviewOps::PrepareReviewPrompt => execute_prepare_review_prompt(inputs), + ReviewOps::ResolveAuthContract => execute_resolve_auth_contract(inputs), ReviewOps::ParseReviewResponse => execute_parse_review_response(inputs), ReviewOps::MergeOutputs => execute_merge_outputs(inputs), ReviewOps::HashFinding => execute_hash_finding(inputs), @@ -376,6 +392,24 @@ fn execute_prepare_review_prompt( .ok() } +fn execute_resolve_auth_contract( + inputs: HashMap, +) -> Result, ExecError> { + let provider = require_str(&inputs, "provider")?; + let intent = ReviewScopeContract::new(provider).credential_intent(); + intent + .validate() + .map_err(|e| ExecError::new(format!("invalid review credential contract: {e}")))?; + + OutputMap::new() + .str("service", intent.service) + .str("scheme", intent.scheme) + .str("header_name", intent.header_name) + .str_list("required_scopes", intent.required_scopes) + .bool("interactive_allowed", intent.interactive_allowed) + .ok() +} + fn execute_parse_review_response( inputs: HashMap, ) -> Result, ExecError> { @@ -777,6 +811,44 @@ mod tests { assert!(question.contains("web application backend")); } + #[test] + fn test_resolve_auth_contract_openai_includes_review_scope() { + let mut inputs = HashMap::new(); + inputs.insert("provider".to_string(), Value::Str("openai".to_string())); + + let result = ReviewOps::ResolveAuthContract + .execute(inputs) + .expect("resolve_auth_contract should succeed for openai"); + + assert_eq!( + result.get("service").and_then(Value::as_str), + Some("openai") + ); + assert_eq!(result.get("scheme").and_then(Value::as_str), Some("bearer")); + let scopes = result + .get("required_scopes") + .and_then(Value::as_str_list) + .expect("required_scopes should be present"); + assert!(scopes.iter().any(|s| s == "llm:chat_completion")); + assert!(scopes.iter().any(|s| s == "review:code_review")); + } + + #[test] + fn test_resolve_auth_contract_anthropic_header() { + let mut inputs = HashMap::new(); + inputs.insert("provider".to_string(), Value::Str("anthropic".to_string())); + + let result = ReviewOps::ResolveAuthContract + .execute(inputs) + .expect("resolve_auth_contract should succeed for anthropic"); + + assert_eq!(result.get("scheme").and_then(Value::as_str), Some("header")); + assert_eq!( + result.get("header_name").and_then(Value::as_str), + Some("x-api-key") + ); + } + #[test] fn test_parse_review_response_valid() { let response = r#"{ diff --git a/lib/tools/gist/Cargo.toml b/lib/tools/gist/Cargo.toml index 47dafd5acd0..cdc4f1c11f9 100644 --- a/lib/tools/gist/Cargo.toml +++ b/lib/tools/gist/Cargo.toml @@ -39,7 +39,3 @@ path = "../../../target/codegen/bin/gist-diff/main.rs" [[bin]] name = "gunbc-gist-recent" path = "../../../target/codegen/bin/gist-recent/main.rs" - -[[bin]] -name = "gunbc-gist-auth-doctor" -path = "../../../target/codegen/bin/gist-auth-doctor/main.rs" diff --git a/lib/tools/gist/src/auth_doctor.rs b/lib/tools/gist/src/auth_doctor.rs deleted file mode 100644 index 15a7ef46d1a..00000000000 --- a/lib/tools/gist/src/auth_doctor.rs +++ /dev/null @@ -1,35 +0,0 @@ -//! Auth doctor graph for gist credential setup diagnostics. -//! -//! Delegates to the provider-neutral cloud auth doctor in cloud-ops, -//! parameterized with the gist credential intent. - -use gunbc_ir::transport::gist::GistRequest; -use gunbc_ir::{BuilderError, Dag}; -use gunbc_lib_cloud_ops::auth_doctor::{build_cloud_auth_doctor_graph, CloudAuthDoctorOp}; - -pub fn build_gist_auth_doctor_graph( - runtime_hint: Option, -) -> Result, BuilderError> { - let intent = GistRequest::new().credential_intent(); - build_cloud_auth_doctor_graph(intent, runtime_hint) -} - -#[cfg(test)] -mod tests { - use super::*; - use gunbc_exec::{execute_with_mode, ExecutionMode}; - use gunbc_ir::Value; - - #[test] - fn test_auth_doctor_graph_runs() { - let dag = build_gist_auth_doctor_graph(None).expect("auth doctor graph should build"); - let log = execute_with_mode(&dag, ExecutionMode::DryRun(Default::default())) - .expect("auth doctor graph should execute"); - let report = log.get("auth_report").expect("auth_report should run"); - assert!(report.outputs.contains_key("required_scopes")); - assert_eq!( - report.outputs.get("service"), - Some(&Value::Str("github".to_string())) - ); - } -} diff --git a/lib/tools/gist/src/graph.rs b/lib/tools/gist/src/graph.rs index 462a52e48e0..ddf71ca45dd 100644 --- a/lib/tools/gist/src/graph.rs +++ b/lib/tools/gist/src/graph.rs @@ -23,7 +23,7 @@ use gunbc_ir::{ WorkflowSignature, }; use gunbc_lib_cloud_ops::{ - build_cloud_secret_manager_credential_graph_from_config, default_local_dev_config, CloudOps, + build_cloud_secret_manager_credential_graph_from_config, graph_cloud_config, CloudOps, CloudSecretManagerGraphOp, }; use gunbc_lib_gist_ops::GistOps; @@ -541,7 +541,7 @@ pub fn build_gist_graph( extensions: Vec, public: bool, ) -> Result, BuilderError> { - build_gist_graph_with_config(mode, extensions, public, default_local_dev_config()) + build_gist_graph_with_config(mode, extensions, public, graph_cloud_config()) } /// Build the gist graph with an explicit `CloudSecretConfig`. @@ -617,8 +617,9 @@ pub fn build_gist_graph_with_config( &[&cloud_env, &resolve_auth], )?; - let cloud_subdag = - lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config(&cloud_config)); + let cloud_subdag = lift_cloud_dag(build_cloud_secret_manager_credential_graph_from_config( + &cloud_config, + )); let cloud_credential = builder.add_node_after(Node::subdag("cloud_credential", cloud_subdag), &bind_secret)?; @@ -706,19 +707,31 @@ pub fn build_gist_graph_with_config( &render_markdown, )?; + // Node: ScopePreflight (PURE - fails fast on invalid/empty scopes) + let scope_preflight = builder.add_node_after( + Node::opaque( + "scope_preflight", + vec![list("required_scopes", "String")], + vec![scalar("scope_verified", "Bool")], + GistGraphOp::Cloud(CloudSecretManagerGraphOp::Cloud(CloudOps::ScopePreflight)), + ), + &resolve_auth, + )?; + // Node: ExecuteGist (BOUNDARY - actual I/O) - let execute_gist = builder.add_node_after( + let execute_gist = builder.add_node_after_all( Node::opaque( "execute_gist", vec![ scalar("request", "TransportRequest"), scalar("skip", "Bool"), + optional("scope_verified", "OptionalBool"), resource("credential", "Credential", AccessMode::Read), ], vec![scalar("response", "TransportResponse")], GistGraphOp::Transport(TransportOps::Execute), ), - &prepare_gist_request, + &[&prepare_gist_request, &scope_preflight], )?; // Node: ParseGistResponse (PURE - extracts URL) @@ -779,6 +792,10 @@ pub fn build_gist_graph_with_config( resolve_auth.out("interactive_allowed"), cloud_credential.in_port("interactive_allowed"), )?; + builder.add_edge( + resolve_auth.out("required_scopes"), + cloud_credential.in_port("required_scopes"), + )?; builder.add_edge( cloud_env.out("request_url"), cloud_credential.in_port("request_url"), @@ -791,6 +808,10 @@ pub fn build_gist_graph_with_config( resolve_auth.out("required_scopes"), prepare_gist_request.in_port("required_scopes"), )?; + builder.add_edge( + resolve_auth.out("required_scopes"), + scope_preflight.in_port("required_scopes"), + )?; builder.add_edge( cloud_env.out("request_token"), cloud_credential.in_port("request_token"), @@ -810,6 +831,10 @@ pub fn build_gist_graph_with_config( prepare_gist_request.out("skip"), execute_gist.in_port("skip"), )?; + builder.add_edge( + scope_preflight.out("scope_verified"), + execute_gist.in_port("scope_verified"), + )?; builder.add_edge( cloud_credential.out("credential"), execute_gist.in_port("res:credential"), diff --git a/lib/tools/gist/src/graph_mock.rs b/lib/tools/gist/src/graph_mock.rs index 33bc43ca5db..6ac8e2d4b73 100644 --- a/lib/tools/gist/src/graph_mock.rs +++ b/lib/tools/gist/src/graph_mock.rs @@ -345,7 +345,8 @@ fn gist_mock_spec(mode: &GistMode) -> MockSpec { ) .skip_node_example("cloud_env") .skip_node_example("cloud_credential") - .skip_node_example("bind_secret"); + .skip_node_example("bind_secret") + .skip_node_example("scope_preflight"); // Mode-specific node examples match mode { @@ -538,34 +539,6 @@ pub fn gist_recent_mock_spec() -> MockSpec { gist_mock_spec(&GistMode::Recent) } -/// Mock spec for auth doctor mode (gist-auth-doctor). -#[gunbc_testgen_registry_macros::resource_test_target( - skip, - name = "gist-auth-doctor", - builder = "crate::auth_doctor::build_gist_auth_doctor_graph(None).unwrap()" -)] -#[gunbc_testgen_registry_macros::testgen_target( - name = "gist-auth-doctor", - output = "lib/tools/gist/src/generated_tests_auth_doctor.rs", - module = "gist_auth_doctor_generated_tests", - builder = "crate::auth_doctor::build_gist_auth_doctor_graph(None).unwrap()", - tool = "gist-auth-doctor", - no_boundary_tests, - window_max_nodes = 1 -)] -pub fn gist_auth_doctor_mock_spec() -> MockSpec { - MockSpec::new("gist-auth-doctor") - .boundary( - "cloud_status", - "status", - Value::Str("Cloud env (gcp/github): OK (mock)".into()), - ) - .boundary("auth_report", "service", Value::Str("github".into())) - .boundary("auth_report", "ready", Value::Bool(true)) - .skip_node_example("cloud_status") - .skip_node_example("auth_report") -} - /// Mock spec for testing gist with file system lock simulation. /// /// Use this when testing tools that acquire file locks before reading. diff --git a/lib/tools/gist/src/lib.rs b/lib/tools/gist/src/lib.rs index be1eff7ea51..c5ca4608a71 100644 --- a/lib/tools/gist/src/lib.rs +++ b/lib/tools/gist/src/lib.rs @@ -27,15 +27,11 @@ //! Mock specs are in `graph_mock.rs` for test generation. #![deny(dead_code)] -pub mod auth_doctor; pub mod graph; pub mod graph_mock; // Re-export public API -pub use auth_doctor::build_gist_auth_doctor_graph; -// Re-export CloudAuthDoctorOp for backwards compatibility (used by graph_mock.rs). -pub use gunbc_lib_cloud_ops::auth_doctor::CloudAuthDoctorOp; pub use graph::{ build_gist_graph, build_gist_graph_with_config, build_read_file_body_dag, gist_signature, GistGraphOp, GistMode, @@ -98,22 +94,6 @@ pub fn gist_diff_tool() {} )] pub fn gist_recent_tool() {} -#[gunbc_tool_registry_macros::tool_target( - name = "gist-auth-doctor", - crate_name = "gunbc-gist", - description = "Diagnose auth/credential setup for gist workflows", - builder = "build_gist_auth_doctor_graph", - args = "if runtime_hint.trim().is_empty() { None } else { Some(runtime_hint.clone()) }", - import = "use gunbc_gist::build_gist_auth_doctor_graph;", - mock_spec = "gunbc_gist::graph_mock::gist_auth_doctor_mock_spec()", - package = "gist", - binary = "gist-auth-doctor", - entrypoints = r#"[{"port_name":"runtime_hint","type_id":"String","short":"r","default":"","help":"Override runtime for diagnosis (github|metadata|local)","make_var":"RUNTIME"}]"#, - has_invocation, - returns_result -)] -pub fn gist_auth_doctor_tool() {} - // ============================================================================ // DagSpec Registry Helpers // ============================================================================ diff --git a/lib/tools/gist/tests/integration.rs b/lib/tools/gist/tests/integration.rs index 5d6ea27729f..2ad9dc8010b 100644 --- a/lib/tools/gist/tests/integration.rs +++ b/lib/tools/gist/tests/integration.rs @@ -68,9 +68,8 @@ fn mock_env(mocks: &mut BoundaryMocks) { let cloud_config = CloudSecretConfig { provider: CloudProviderKind::Gcp, - runtime: CloudRuntimeKind::GitHubActions, - audience: "projects/123/locations/global/workloadIdentityPools/github/providers/gha" - .to_string(), + runtime: CloudRuntimeKind::LocalDev, + audience: "local-dev".to_string(), project_or_account: "mock-secrets".to_string(), secret: CloudSecretRef { prefix: "ci-".to_string(), @@ -109,6 +108,45 @@ fn mock_env(mocks: &mut BoundaryMocks) { ])); mocks.set_value("cloud_credential", "credential", credential); mocks.set_value("cloud_credential", "expires_in", Value::Int(3_600)); + // local_auth_upsert sub-DAG mocks (local-dev path) + let adc_path = "/tmp/mock-adc.json"; + let mock_adc_json = serde_json::json!({ + "type": "authorized_user", + "client_id": "mock-client-id.apps.googleusercontent.com", + "client_secret": "mock-client-secret", + "refresh_token": "mock-refresh-token" + }) + .to_string(); + mocks.set_value( + "cloud_credential/gcp_wif_secret/local_auth_upsert/net_env", + "net", + gunbc_primitives::NetworkHandle.into(), + ); + mocks.set_value( + "cloud_credential/gcp_wif_secret/local_auth_upsert/execute_check", + "response", + Value::Response(TransportResponse::File( + gunbc_ir::transport::file::FileResponse::exists_result(adc_path, true), + )), + ); + mocks.set_value( + "cloud_credential/gcp_wif_secret/local_auth_upsert/execute_read_adc", + "response", + Value::Response(TransportResponse::File( + gunbc_ir::transport::file::FileResponse::read_ok(adc_path, mock_adc_json), + )), + ); + mocks.set_value( + "cloud_credential/gcp_wif_secret/local_auth_upsert/execute_oauth2", + "response", + Value::Response(TransportResponse::Rest( + gunbc_ir::transport::RestResponse::ok(serde_json::json!({ + "access_token": "mock-local-token", + "expires_in": 3599, + "token_type": "Bearer" + })), + )), + ); mocks.set_value( "cloud_credential/gcp_wif_secret/net_env", "net",