feat: Add secret resolution system design for managed secrets

[induwara-yaala]

Description

Add secret resolution system design for managed secrets

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Test update
• CI/CD update
• Other (please describe):

Changes Made

Add secret resolution system design for managed secrets

Testing

• Unit tests pass locally
• Integration tests pass locally
• Manual testing completed
• New tests added for changes

Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

[Copilot]

Copilot review overview

🟡 Changes recommended

The design has unresolved scope, API, security, concurrency, error-contract, and deployment requirements.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 6 · 1

Open (8)

• Key validation permits unsafe environment variable access · New
• Public types reject documented None default behavior · New
• Cache locking does not ensure provider thread safety · New
• cache_ttl lacks range validation and zero/negative semantics · New
• Terraform opt-in variable and resource gating are underspecified · New
• Example scope omits required AWS containerized deployment · New
• Design narrows approved requirements without documented scope change · New
• Stage 1 design includes premature implementation details · New

What changed in this PR

This PR adds a Stage 1 design for managed secret resolution with pluggable providers, SSM, environment fallback, caching, and AWS deployment wiring.

Changes:

• Defines manager/provider APIs, configuration, injection, and error handling.
• Specifies caching, IAM/Terraform integration, examples, and testing.
• Documents scope, non-goals, and open questions.

File	Description
docs/​specs/​749-secret-resolution/​design.md	Stage 1 managed-secret resolution system design

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Critical IAM scope, deployment secret injection, lock reproducibility, and validation issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 9 · 1 · 4

Open (14)

• Scope SSM IAM ARN to the current AWS account · New
• Scope SSM IAM ARN to the current AWS account · New
• Scope SSM IAM ARN to the current AWS account · New
• Scope SSM IAM ARN to the current AWS account · New
• Scope SSM IAM ARN to the current AWS account · New
• Scope SSM IAM ARN to the current AWS account · New
• Restore API key injection or provision SSM parameter · New
• Restore API key injection or provision SSM parameter · New
• Regenerate lockfile for clean-checkout reproducibility · New
• Use full-string matching for secret key validation · New
• Update stale Terraform module version reference · New
• Complete documentation and skills synchronization · New
• Confirm design approval before advancing staged spec · New
• Update stale release-sequencing version reference · New

Resolved since last review (8)

• Key validation permits unsafe environment variable access
• Design narrows approved requirements without documented scope change
• Example scope omits required AWS containerized deployment
• Terraform opt-in variable and resource gating are underspecified
• cache_ttl lacks range validation and zero/negative semantics
• Cache locking does not ensure provider thread safety
• Public types reject documented None default behavior
• Stage 1 design includes premature implementation details

[amithad]

Overall: solid, spec-driven implementation. The secret/ package, the _SecretConfig block, the Terraform wiring and the tests match the approved design and the house patterns (pluggable by default, justified config fields, classes not scripts, secret/ imports core only). The main gap is that the spec set no longer describes what shipped for the examples and CI. CI: all 64 checks pass.

Spec verdict (reviewed before the code)

• design.md, spec.md and plan.md all reviewed. Every path:line, default and count cited against the base branch verified (the 34 main.tf files, every config.py, Terraform module and CI line reference), with one exception: the "70 models" count is 68 (nit inline).
• Cross-document consistency holds: spec.md covers every design.md requirement and plan.md covers every spec.md component. The one break is the examples/CI decision, which changed during implementation and is now recorded in the code, READMEs and skills but in none of the three documents (inline on spec.md:811).
• Design approval staging and the scope narrowing vs #749 were already raised by Copilot and are not repeated here.

Spec conformance (implementation)

• Implemented as specified: SecretProvider ABC, concrete SecretManager with current()/reset(), SecretCache (monotonic TTL, lock-free reads, guarded eviction), SecretProviderFactory with require_extra + resolve_dotted, env/aws_ssm providers, SecretProviderContract; _SecretConfig with exactly the three justified fields and no enabled/manager type; no module-level functions and nothing outside secret/ imports it; ssm_enabled on both roots, count-gated, account-scoped ssm:GetParameter per tier, AK_SECRET__PREFIX injected, provider type never injected; docs and skills sync done except the new dev skill.
• Deviated (inline): the two AWS examples drop the OPENAI_API_KEY injection instead of keeping it optional; CI now seeds SSM where the spec says "no CI change"; the ak-dev-new-secret-provider skill required by plan.md Iteration 9 is absent.
• Beyond the spec: examples/cli/openai_secret and its e2e matrix entry (useful and green in CI). Worth one line in spec.md Examples and plan.md Iteration 7 so the plan matches the PR.

Findings: 0 blockers, 4 suggestions, 2 nits, 1 question (all inline).

Not anchorable to a diff line

• [suggestion] Tests: spec.md Testing and plan.md Iteration 8 step 1 promise "the environment wins for every provider" and "an empty variable reaches aws_ssm" driven through the manager with the fake SSM client recording zero or one get_parameter call. Only the dict-provider variants exist (test_environment_wins_over_cache_and_provider, test_empty_environment_variable_is_a_miss). Two small tests in test_secret_manager.py using the _FakeSSMClient fixture would close it.
• [suggestion] Docs-site React pages: README, intro.md and the docs pages now list the capability, but docs/src/pages/index.tsx (ak-add-capabilities pills) and features.tsx (Core Capabilities cards) do not. Scheduling and threads have no card either, so this is optional; a Secrets pill on the ak-add-capabilities entry is the cheapest consistency fix.

Positives

• _KEY_PATTERN.fullmatch and the ${var.account_id}-scoped ARNs address the earlier Copilot findings.
• The _DictSecretProvider test double is itself held to SecretProviderContract; the cache eviction race has a deterministic test; no secret value can reach a log record or exception message; the CI seed passes the value via --cli-input-json file://, never argv.

Skipped as duplicates of existing feedback: design approval staging (spec.md:17), scope narrowing vs #749 (design.md:167), Stage-1 implementation detail (design.md:88).