feat(credentials): JIT credential dispensing per tool call

[naveen-kurra]

Closes #215.

Summary

• New forge-core/credentials/ package: Provider / Credential interfaces, Registry, Injector, per-invocation Handle.
• Reference providers:
  • static — passthrough env/headers (bootstrap + tests).
  • sts_assume_role — AWS STS AssumeRole with hand-rolled SigV4 (SDK-free, matches Forge's Bedrock signer pattern). Enforces AWS 15m–12h duration bounds; supports external_id + inline session policy.
• ForgeConfig.Credentials []CredentialSpec — operators declare per-tool JIT specs in forge.yaml. Empty → pre-R9 behavior.
• Runner startup resolves each spec and wires the injector into CLIExecuteTool.WithCredentialInjector(...).
• cli_execute materializes credentials on every Execute call, merges env into the subprocess, and revokes on defer.
• New audit events: credential_issued, credential_revoked, credential_failed — payloads carry provider/tool/binary/ttl/env_keys, never the credential material itself.

Acceptance criteria (from #215)

• credentials.Credential + credentials.Provider interfaces in forge-core/credentials/
• sts_assume_role provider ships as the reference implementation
• Skill can declare per-tool credential requirements (via forge.yaml)
• BeforeToolExec-equivalent materializes the credential and scopes the tool's env accordingly (via Injector.Materialize inside cli_execute.Execute)
• credential_issued + credential_revoked audit events with provider, TTL, scope
• Tests: mock STS server; verify per-tool creds materialize, get injected, and are revoked
• Docs: docs/security/least-privilege-credentials.md

Test plan

• go test ./forge-core/credentials/... — interfaces, registry, static provider, STS mock server (request form, SigV4 shape, error paths, duration bounds)
• go test ./forge-cli/tools/... — end-to-end cli_execute injection: env visible in subprocess, audit events fire, binary-scoping works, no-injector fallback is a no-op
• go test ./forge-core/... ./forge-cli/... — full sweep green
• gofmt -w + golangci-lint run all clean

[initializ-mk]

Reviewed the full diff, hand-traced the SigV4 path, verified the env-injection scoping empirically, and ran the credentials + cli_execute suites (all green). This is a clean, well-tested design with no blocking correctness bug — but I'm requesting changes to close one security-critical test gap and to fold the follow-ups into this PR before merge.

What's correct (for the record)

• SigV4 canonicalization is correct — method / URI / empty query / canonical-headers (trailing \n) / signed-headers / payload-hash; the required blank line falls out of the trailing newline + join; sts scope, session-token header, and signing-key derivation all match spec.
• Injection is properly scoped — creds go into cmd.Env (child only), never os.Setenv; minted per call; GC'd after. Nil-injector is a clean no-op.
• Audit stays metadata-only — events carry env_keys / header_keys (names), never values.

Requested changes (this PR)

1. (Security-critical) Regression-test the JIT override; don't rely solely on Go's exec dedup

The docs promise JIT creds "override any same-name values from the operator's static passthrough." That only holds because Go's os/exec dedups env keeping the last entry — I confirmed it (K=source-broad then K=jit-scoped -> child sees jit-scoped on go1.26), and buildEnv does pass EnvPassthrough through, so the conflict path is real (an operator who lists AWS_ACCESS_KEY_ID in env_passthrough). But no test asserts it. If that runtime behavior changed, or a refactor prepended instead of appended, the subprocess would silently keep the broad source creds — a silent privilege escalation.

• Add a test that seeds a conflicting key via buildEnv/env_passthrough and asserts the JIT value wins in the subprocess.
• Prefer belt-and-suspenders: explicitly strip conflicting keys from cmd.Env before appending the JIT env, rather than trusting exec dedup.

2. Cache STS credentials within their TTL (no per-call AssumeRole)

Credential.Materialize does a full STS AssumeRole on every tool invocation, even though the interface doc contemplates caching. An agent that runs aws N times makes N AssumeRole calls -> latency before each exec + account-level throttling. Since the provider ignores args (no per-action scope-down), every call for a given spec yields an equivalent credential, so caching within the TTL is safe and strictly better.

• Cache the materialization per spec and reuse until near expiry (leave a skew), re-minting only when expired.

3. Wire the JIT Headers path into http_request (the HTTP/API half of R9)

Materialization.Headers / Handle.Headers() are defined but have no consumer — only cli_execute calls the injector. As-is, JIT credentials only reach subprocess env, not HTTP/API endpoints, so the Headers surface is dead code and the PR implies more coverage than it delivers.

• Wire the injector into http_request (materialize on the call, inject Handle.Headers() into the outbound request, Close after) so per-call credentials work for API endpoints too — the natural sibling of the cli_execute path.

4. Make credential_revoked honest about self-expiring creds

STS and static set Revoke == nil, so Close emits credential_revoked even though nothing was invalidated (the STS token is still live until its 15m expiry). An operator can't distinguish "hard-revoked" from "tool finished, token self-expires later."

• Add a field (e.g. revoked: bool / self_expiring: true) so the audit reflects what actually happened.

5. Minor

• truncate slices bytes, not runes (comment says runes) — can emit invalid UTF-8 in an error string; slice on runes or note it.
• types (a widely-imported package) now imports credentials — a coupling that will drag credentials' future deps into everything importing types; consider keeping CredentialSpec minimal / breaking the direction if credentials grows non-stdlib deps.
• Injected creds land in subprocess stdout -> the LLM (the JIT test asserts AWS_ACCESS_KEY_ID=… in stdout). Fine and disclosed, but cross-reference the #221 deny_output redaction as the mitigation in the threat-model doc.

Explicitly OUT of scope (do not add here)

• Vault dynamic-secrets provider — that belongs to Vault's capability/role, not this PR. The plugin interface is the right seam for it later; no need to build it now. (Same for k8s TokenRequest / other backend providers — the interface is enough for this PR.)

Verdict

Request changes. #1 (override test) and #3 (headers wiring) are the ones I'd block on — #1 because a silent privilege downgrade is the worst failure mode here, #3 because the HTTP story is currently implied but not delivered. #2 (caching) matters as soon as this sees real aws usage. Once those land, this is a solid R9 foundation.

[naveen-kurra]

Thanks for the review @initializ-mk — all five points addressed in c7c6620.

🔴 #1 JIT override + regression test — Belt-and-suspenders. cli_execute now runs a stripEnvKeys pass over cmd.Env before appending the JIT env, so a same-name env_passthrough entry gets removed rather than duplicated. TestCLIExecute_JITOverridesEnvPassthrough seeds AWS_ACCESS_KEY_ID=AKIASOURCEBROAD via env_passthrough, runs env under a JIT provider stamping AKIAJITSCOPED, and asserts (a) the JIT value is present, (b) the source value is absent, (c) exactly one AWS_ACCESS_KEY_ID= line in stdout. Pre-fix, the test would have shown two lines and relied on exec dedup for correctness.

🔴 #3 http_request headers wiring — Filled the dead-code path. httpRequestTool.WithCredentialInjector(inj) mirrors cli_execute's pattern; the runner passes the same injector via new builtins.Options{HTTPCredentialInjector: credInjector}. On every http_request call the injector materializes, Handle.Headers() merges onto the outbound request, and Close+audit fire. LLM-authored headers of the same name are OVERRIDDEN by JIT — TestHTTPRequest_JITHeadersOverrideLLMHeaders proves it. Tool-scoping preserved: a cli_execute-scoped spec doesn't fire on http_request calls.

🟠 #2 STS caching — Per-Credential cache keyed off the STS-issued Expiration minus a 60s skew. Materialize fast-paths cached bytes under a mutex; on miss it AssumeRoles once and populates. TestSTSProvider_MaterializeCachesWithinTTL — 5 Materialize calls, 1 STS hit. TestSTSProvider_ExpiredCacheReMints — advance a fake clock past the skew and observe 2 STS calls total.

🟠 #4 credential_revoked honesty — Event now carries revoked: bool + self_expiring: bool. STS + static emit revoked=false, self_expiring=true (nothing was invalidated at the source). A future Vault provider with Materialization.Revoke != nil will emit revoked=true, self_expiring=false. If revoke fails, the correction flips both to false with an error field. Tests: TestHandleClose_SelfExpiringWhenNoRevoke + TestHandleClose_HardRevokeWhenProviderRevokes.

🟡 Minor

• truncate now slices runes, not bytes — no invalid-UTF-8 in error messages.
• Coupling note added to types.ForgeConfig.Credentials: the base credentials package MUST stay stdlib-only; provider deps live under credentials/<name>/. Comment references your review.
• Threat-model doc now cross-references R4a deny_output (R4a (governance): generalize MODIFY decision beyond cli_execute output #209) as the mitigation for the "subprocess dumps env → LLM sees creds" footgun, with an example SKILL.md redact pattern list.

Out of scope confirmed — Vault dynamic-secrets provider explicitly deferred; the Provider interface is the seam and it can land in a follow-up.

Full go test ./forge-core/... ./forge-cli/... sweep green; gofmt -w + golangci-lint run clean.

[initializ-mk]

Request changes — the four fixes from the earlier round are all correct and tested, but standing up a live agent to exercise the scenarios surfaced a blocker: the credentials block cannot be configured from forge.yaml at all.

Fixes from the prior review — verified correct

Checked against the fix commit (builds clean; credentials + tools suites green):

• Env override strip — cli_execute now stripEnvKeys(cmd.Env, jitEnv) before appending the JIT env, so a JIT key deterministically overrides a passthrough of the same name rather than relying on exec dedup order. The security-critical TestCLIExecute_JITOverridesEnvPassthrough is present.
• STS cache — thread-safe fast/slow path, reuses the minted credential until expireAt - 60s skew, deep-clones on return. No more AssumeRole per tool call.
• http_request wiring — the tool now materializes and req.Header.Sets the JIT headers and defers revoke; the Headers path is finally consumed.
• Honest revoked event — credential_revoked carries revoked / self_expiring (false/true for static + STS self-expiring creds).

Blocker: credentials[].spec is unparseable in forge.yaml

forge validate on the documented format fails at config load:

credentials:
  - tool: cli_execute
    binary: env
    provider: static
    spec:
      env: { JIT_TOKEN: jit-secret-abc123 }   # the shape shown in docs / config.go

Error: parsing forge config: yaml: unmarshal errors:
  cannot unmarshal !!map into json.RawMessage

CredentialSpec.Spec (forge-core/credentials/credentials.go) is a json.RawMessage ([]byte) tagged yaml:"spec" with no custom UnmarshalYAML. yaml.v3 has no built-in path from a YAML node into json.RawMessage, so it errors. I tried both authoring forms:

• documented map -> cannot unmarshal !!map into json.RawMessage
• JSON string (spec: '{"env":{...}}') -> cannot unmarshal !!str into json.RawMessage

Both fail. The operator-facing feature -- declare per-tool JIT specs in forge.yaml -- does not work through its only interface. The unit tests miss it because they construct CredentialSpec{Spec: json.RawMessage(...)} directly in Go, bypassing YAML entirely; nothing exercises ParseForgeConfig on a credentials block.

Fix: add UnmarshalYAML(*yaml.Node) to CredentialSpec that decodes the outer fields and re-encodes the spec sub-node to JSON (yaml.Node -> map[string]any -> json.Marshal -> RawMessage). This exact pattern already lives in the repo -- BinRequirement.UnmarshalYAML at forge-skills/contract/types.go:118. Please also add a config round-trip test (ParseForgeConfig on a credentials: block -> Injector builds successfully) so this can't regress -- that's the gap that let it through.

How I verified

Built the PR binary and a demo agent (jit-demo): a static provider injecting JIT_TOKEN into cli_execute's env and X-Jit-Auth into http_request, with a driver that reads the verdict from the audit log (credential_issued / credential_revoked with revoked:false, and JIT_TOKEN=... in the subprocess output). The harness is ready; it's currently blocked because the agent can't start with a credentials block. Once the UnmarshalYAML fix lands I'll re-run it to confirm the env-injection and the new header-injection paths end to end.

[initializ-mk]

Follow-up on the parse blocker: the shipped docs are the repro. Every credentials example in docs/security/least-privilege-credentials.md fails to load with the PR binary -- both providers, verbatim:

• sts_assume_role (lines 35-42): forge validate -> cannot unmarshal !!map into json.RawMessage
• static (lines 45-51): same error on the spec: map

So a user copy-pasting either example from this PR's own documentation gets a startup error, not a working agent. Same root cause as the review -- CredentialSpec.Spec is json.RawMessage with no UnmarshalYAML.

One extra thing for whoever implements the UnmarshalYAML fix: it can't stop at "the map now parses." After the yaml.Node -> map -> json.Marshal round-trip, the scalar leaves become JSON strings -- so duration: 15m (STS) and ttl: 1h (static) arrive at the provider as the JSON strings "15m" / "1h". time.Duration doesn't unmarshal from a Go-duration string via encoding/json out of the box, so the provider spec decoders need to accept a string there (or the failure just moves from "config won't parse" to "provider rejects the spec"). Please add round-trip tests for both providers -- ParseForgeConfig on the doc's static and sts_assume_role blocks -> Injector builds -> the duration/ttl value decodes to the expected time.Duration -- not just static.

[naveen-kurra]

Fix in 901d2a4 — UnmarshalYAML(*yaml.Node) on CredentialSpec per the pattern you suggested (mirrors BinRequirement.UnmarshalYAML).

End-to-end verified live on my machine, same shape as your harness:

Built the binary from the fix commit, dropped this forge.yaml:

```yaml
agent_id: jit-demo
version: 0.1.0
framework: forge
entrypoint: main.py
credentials:

• tool: cli_execute
  binary: env
  provider: static
  spec:
  env:
  JIT_TOKEN: jit-secret-abc123
  AWS_ACCESS_KEY_ID: AKIAJITSCOPED
  ttl: 15m
• tool: http_request
  provider: static
  spec:
  headers:
  X-Jit-Auth: yes-jit-was-here
  ```

Results:

• `forge validate` → Validation passed (was: cannot unmarshal !!map into json.RawMessage)
• `forge run` startup: JIT credential injector wired specs=2
• Driver program (parse yaml → NewInjector → cli_execute+http_request):
  • env subprocess stdout contains JIT_TOKEN=jit-secret-abc123 + AWS_ACCESS_KEY_ID=AKIAJITSCOPED ✓
  • http_request outbound: X-Jit-Auth: yes-jit-was-here observed at the receiving httptest server ✓
  • Audit events: credential_issued+credential_revoked per tool call, both revoked:false, self_expiring:true for static creds (the honesty fix from the prior round holds).

Regression tests at forge-core/types/config_credentials_test.go:

• `TestParseForgeConfig_CredentialsBlock` — full YAML round-trip on the two-spec fixture; asserts outer fields + JSON-encoded spec substrings (order-stable).
• `TestParseForgeConfig_Credentials_BuildsInjector` — end-to-end: ParseForgeConfig → NewInjector succeeds. Catches breaks in either half of the round-trip. Static provider registered via blank import so the assertion is meaningful.
• `TestParseForgeConfig_CredentialsEmptySpec` — spec-less entry accepted (some providers infer everything from env).
• `TestCredentialSpec_JSONRoundTripIndependence` — direct JSON round-trip still works; UnmarshalYAML doesn't regress the JSON path used by OCI packaging + existing tests.

Full `go test ./forge-core/... ./forge-cli/...` sweep + `gofmt -w` + `golangci-lint run` all clean.

[initializ-mk]

Approving — the 901d2a4 fix resolves my second-round blocker, and I re-validated the whole feature live end to end (twice) against a binary built from the branch head.

Blocker resolved

CredentialSpec.UnmarshalYAML is the right fix and mirrors the in-repo BinRequirement.UnmarshalYAML pattern: decode the outer fields via a type-alias that swaps Spec for a yaml.Node, then Decode -> json.Marshal so providers keep receiving canonical json.RawMessage bytes; empty spec: (Kind == 0) decodes to nil for env-inferring providers. The four regression tests in forge-core/types/config_credentials_test.go (block round-trip, parse -> NewInjector, empty spec, JSON-path independence) lock it in. All green.

The duration/ttl-as-string concern from my follow-up is a non-issue: both sts.Duration and static.TTL are typed as plain string (STS calls time.ParseDuration on it), so the re-encoded "15m" decodes cleanly.

Live end-to-end validation

Built a binary from 901d2a4, ran a demo agent with a static provider injecting JIT_TOKEN into cli_execute/env and X-Jit-Auth into http_request, driven by a real model:

• forge validate passes on the demo config AND on the verbatim sts_assume_role example from docs/security/least-privilege-credentials.md (both failed before the fix).
• Startup logs JIT credential injector wired specs=2.
• Per tool call: credential_issued + credential_revoked pair, revoked:false, self_expiring:true (the honesty fix from the prior round holds).
• Injection confirmed at the destination: JIT_TOKEN=jit-secret-abc123 in the cli_execute subprocess env, and httpbin echoed back X-Jit-Auth on the http_request call (the new header-wiring path). Stable across repeat runs.

Non-blocking follow-up (not gating this PR)

The regression tests cover the static provider round-trip; worth adding an sts_assume_role round-trip too (ParseForgeConfig on the doc's STS block -> NewInjector). It's the same string-typed field so there's no correctness risk today, but it pins the shape the docs advertise. Fine as a fast-follow.

Nice work turning this around.