feat(security): STEP_UP authorization decision (closes #210)

[naveen-kurra]

Closes #210. Governance R4b — adds the third policy decision from the R4 taxonomy: on a high-assurance tool call, if the caller's authentication assurance is insufficient, Forge returns an RFC 9470 step-up challenge instead of running the tool.

Design

• New forge-core/security/stepup/ package: Engine reads per-tool acr requirements from ForgeConfig, checks against auth.Identity.Claims["acr"], returns typed *RequiredError on mismatch.
• Optional acr_hierarchy for "stronger acr satisfies weaker requirement" semantics; default strict-equal comparison.
• Runner registers a BeforeToolExec hook that fires AFTER guardrails (so a guardrail-denied call doesn't produce an unnecessary challenge) and BEFORE tool execution.
• WriteStepUpChallengeOnError translates the typed error into HTTP 401 with WWW-Authenticate: Bearer error="step_up_required", acr_values="<acr>" per RFC 9470 §3.
• PolicyResult.RequiredAcr field + StepUp() constructor populate the existing DecisionStepUp enum slot from R4a (feat(guardrails): generalize MODIFY policy decision beyond cli_execute #221).

Config

```yaml
security:
step_up:
enabled: true
tools:
cli_execute: acr:mfa
http_request: acr:mfa
acr_hierarchy: # optional
- acr:password
- acr:mfa
- acr:hardware
```

Fail-loud on startup: enabled: true with empty tools list is rejected; a tool acr not in the hierarchy (typo) is rejected.

Audit event

New auth_step_up_required event with fields {tool, required_acr, presented_acr, reason}. No token bytes, no full claim maps.

Wire coverage

• POST /tasks/send REST — HTTP 401 with RFC 9470 challenge header ✅
• JSON-RPC + SSE — the typed error surfaces in the response body but doesn't translate to an HTTP 401 header on these transports; documented as follow-up. Deployments where these are the primary surface can pair step-up with a strict guardrail deny so the tool never runs even without the transport-native signal.

Acceptance criteria (from #210)

• DecisionStepUp returnable from any guardrail hook (via runtime.StepUp(requiredAcr, reason))
• HTTP 401 with RFC 9470 step-up challenge on StepUp (REST path)
• Auth middleware validates acr claim on the retry (auth package already carries Claims map; Engine.Check reads acr from it)
• auth_step_up_required audit event with tool, reason, required acr
• Config: per-tool acr requirement + optional hierarchy
• Tests: end-to-end challenge shape + engine unit tests
• Docs: docs/security/step-up-auth.md

Test plan

• go test ./forge-core/security/stepup/... — 12 tests (validation, strict + hierarchy paths, fail-closed, RequiredError unwrap)
• go test ./forge-cli/runtime/... — 4 challenge-writer tests (RFC 9470 format, wrapped-error unwrap, non-step-up passthrough)
• Full sweep go test ./forge-core/... ./forge-cli/... green
• gofmt -w + golangci-lint run clean

[naveen-kurra]

Live-verified end-to-end. Built the binary from the branch tip. Config exercise:

```yaml
security:
step_up:
enabled: true
tools:
cli_execute: acr:mfa
acr_hierarchy: [acr:password, acr:mfa, acr:hardware]
```

Engine.Check() across 5 caller identities:
```
nil identity → deny ✓ blocked: no acr claim presented
no acr claim → deny ✓ blocked: no acr claim presented
acr:password (weaker) → deny ✓ blocked: presented acr "acr:password" does not satisfy required "acr:mfa"
acr:mfa (matches) → allow ✓
acr:hardware (stronger) → allow ✓
```

RFC 9470 challenge shape via WriteStepUpChallengeOnError:
```
HTTP/1.1 401
WWW-Authenticate: Bearer error="step_up_required", acr_values="acr:mfa"
body: {"error":"step_up_required","tool":"cli_execute","required_acr":"acr:mfa","reason":"presented acr "acr:password" does not satisfy required "acr:mfa""}
```

• ✓ Hierarchy: hardware > mfa > password — stronger satisfies weaker requirement, weaker fails.
• ✓ Fail-closed on nil identity and missing acr claim.
• ✓ Challenge header + body carry the required_acr so callers can enroll the right method.
• ✓ Non-step-up errors pass through the writer untouched (fall through to caller's default error handler).

Repro is in /tmp/pr247-demo/ (yaml + Go driver). Ready for merge review.

[initializ-mk]

Request changes — the stepup engine is clean, but there's a critical integration bug: the RFC 9470 401 challenge is never emitted on any transport, including the REST path the PR marks as working. The primary acceptance criterion ("HTTP 401 with RFC 9470 step-up challenge on StepUp") is unmet.

Blocker: the 401 step-up challenge is never sent

The hook correctly returns a typed *stepup.RequiredError, and loop.go propagates it (before tool exec hook: %w). But executeTask swallows it: on any executor.Execute error it sets task.Status = Failed, emits session_end, and returns (task, snap, nil). All five of executeTask's return statements pass nil as the error — none return the executor error.

So the REST handler never reaches the challenge writer:

task, snap, err := r.executeTask(ctx, ...)   // err is ALWAYS nil on tool failure
if err != nil {                               // never true for a step-up denial
    if WriteStepUpChallengeOnError(w, err) { return }   // dead code for step-up
    writeJSON(w, http.StatusInternalServerError, ...)
    return
}
writeJSON(w, http.StatusOK, task)             // returns 200 + a "failed" task body

The caller gets HTTP 200 with a failed-task body, no WWW-Authenticate header, no acr_values. RFC 9470's whole purpose — a machine-readable challenge the client acts on to re-authenticate and retry — is unfulfilled.

Why it slipped through: the 4 challenge-writer tests all call WriteStepUpChallengeOnError directly with a RequiredError, bypassing executeTask. Nothing drives the handler end to end, so the swallow is untested.

Severity nuance: the tool is still correctly blocked — the task fails and the tool never executes, so fail-closed is intact and this is not a security hole. But the feature is non-functional: there's no path to step up and retry.

Fix: have executeTask return the step-up error (detect via stepup.AsRequiredError and return it as the third value, keeping the failed-task store/audit side effects) so the handler's WriteStepUpChallengeOnError(w, err) branch is reachable. Then add an end-to-end handler test: httptest POST /tasks/send that triggers a step-up tool call and asserts 401 + the WWW-Authenticate: Bearer error="step_up_required", acr_values="..." header. Same gap exists on the JSON-RPC/SSE paths (documented as follow-up) — but REST is claimed to work and doesn't.

Correctness — verified good

• Engine is solid. Immutable after construction (no locking), fail-closed on nil identity / no acr, strict-equal and hierarchy comparison modes, Validate catches enabled-with-empty-tools and a tool acr missing from the hierarchy (typos).
• Identity propagation works. WithIdentity (auth middleware) -> req.Context() -> executeTask only adds ctx values (never strips) -> executor.Execute -> hooks.Fire -> auth.IdentityFromContext resolves in the hook.
• Audit fires correctly. auth_step_up_required is emitted inside the hook before the error is swallowed, so the SIEM signal is intact — only the client-facing challenge is lost.

Minor

• Ordering claim. The hook is registered after the library guardrails but before the skill guardrails, so a skill-guardrail-denied call still produces a step-up challenge first — slightly narrower than the "fires AFTER guardrails" rationale in the comment.
• step_up + --no-auth. Identity is always nil, so every step-up tool is permanently gated (and, per the bug above, returns 200-failed rather than a challenge). Worth a startup warning or doc note that step_up requires auth to be meaningful.

The engine design and the RFC 9470 challenge writer are both correct in isolation — the gap is purely that executeTask never lets the error reach the writer. Wire that through with an end-to-end test and this is close.

[initializ-mk]

Approving — commit 7950cee fixes the blocker exactly as recommended, and closes the coverage gap. Re-verified on the branch: build clean, full forge-cli/runtime suite green, all step-up tests pass.

Blocker — 401 challenge never fired — fixed

executeTask now surfaces the step-up error to the handler:

if _, isStepUp := stepup.AsRequiredError(err); isStepUp {
    return task, acc.Snapshot(), err
}
return task, acc.Snapshot(), nil

Verified:

• Correct placement — in the generic "execute failed" block (a step-up RequiredError is not context.Canceled, so it flows here, not the cancellation path), and it runs after the failed-task side effects (store.Put, session_end, invocation_complete), so audit + task state are preserved and the caller can GET /tasks/{id} after re-auth.
• Surgical — non-step-up executor errors still return nil, so ordinary tool failures keep rendering 200 + failed-task. Pinned by TestExecuteTask_NonStepUpErrorStaysAsNil.
• errors.As unwrap works through loop.go's "before tool exec hook: %w" wrap.

Tests — gap closed

step_up_execute_task_test.go:

• ReturnsStepUpError — err propagates, unwraps to RequiredError, side effects fire.
• NonStepUpErrorStaysAsNil — surgical guard.
• StepUpEndToEnd_ChallengeEmitted — drives executeTask -> WriteStepUpChallengeOnError and asserts 401 + WWW-Authenticate (error="step_up_required", acr_values) + JSON body. The exact REST-handler seam.

Follow-up (tracked, non-blocking)

• JSON-RPC / SSE paths still swallow the error (no 401 there) — reaffirmed as documented follow-up. The PR's contract was that REST works, and REST now works.
• Two minors from the first review remain fine to defer: the hook fires after library but before skill guardrails; and step_up + --no-auth permanently gates (identity always nil) — worth a startup warning eventually.

Nice work — the fix is exactly the right shape (surface the typed error, keep the side effects, leave other errors alone) and the end-to-end test mirrors the handler path precisely.