feat(authbridge): credential placeholder swap (hide real tokens from the agent)

[huang195]

Summary

Adds an opt-in credential placeholder swap mode that keeps the real user token out of the agent. On inbound, jwt-validation validates the token, mints an opaque abph_ handle, stores the real token in a process-scoped shared store, and forwards only the handle to the agent. On egress, token-exchange resolves the handle back to the real token before its normal RFC 8693 exchange. A compromised or prompt-injected agent therefore holds only an opaque, audience-scoped handle that is worthless outside the proxy.

This escapes the prior either/or: it preserves user-delegated identity (unlike client-credentials injection) and keeps the secret away from the agent (unlike forwarding the real token).

Design spec: authbridge/docs/superpowers/specs/2026-06-02-credential-placeholder-swap-design.md
Implementation plan: authbridge/docs/superpowers/plans/2026-06-02-credential-placeholder-swap.md

What changed

• New authlib/shared — generic, semantics-free, process-scoped TTL key→value store.
• New authlib/placeholder — handle convention: abph_ prefix, CSPRNG ≥256-bit New(), namespaced Key(), IsPlaceholder().
• pipeline.SharedStore interface + Context.Shared — the seam listeners inject; no global state.
• jwt-validation — new placeholder_mode / placeholder_ttl config (off by default). Mints the handle on the validated allow path; fails closed if no store is wired. Records only a short handle prefix, never the token.
• token-exchange — new resolve_placeholders config (off by default). Resolves the handle to the real token before HandleOutbound; unresolvable handles are denied (fail closed). Non-placeholder bearers are unaffected.
• Inbound header propagation — reverseproxy and extproc (header + body paths) now propagate an inbound Authorization mutation to the agent (previously dropped). forwardproxy exposes the store on the outbound context.
• Wiring — the store is created once and injected into both listeners in authbridge-proxy, authbridge-lite, and authbridge-envoy (mirrors the existing sessions injection).
• Docs — authbridge/docs/plugin-reference.md.

Security properties

• Real token never reaches the agent and is never logged in cleartext.
• Handle is a random ≥256-bit abph_ token, meaningless outside the minting process.
• No off-route leak: token-exchange only writes the Authorization header on a matched exchange route (ActionReplaceToken), so resolving before routing cannot send the real token to a passthrough/unmatched host (regression-tested).
• Fail-closed on every resolve failure (nil store, unknown/expired handle).

Behavior notes

• The two flags are a matched pair: mint without resolve → outbound fail-closed deny; resolve without mint → no-op.
• Passthrough hosts receive the placeholder, not the real token — hosts needing a real credential must be exchange routes.
• The in-memory store works where mint and resolve share a process: the reverse+forward proxy sidecar and single-replica authbridge-envoy (extproc). Multi-replica / shared-waypoint deployments need an external store — a documented future enhancement.

Scope / deferred

• placeholder_ttl is a configured duration (default 1h); binding it to the token's exp is a future enhancement.
• Eviction is lazy-only in v1 (periodic sweep deferred).
• extauthz/waypoint and the external store are out of scope (not compiled into any shipped binary).

Testing

• TDD throughout. New unit tests for the store (incl. TTL with injected clock, -race), placeholder helpers, mint mode (incl. nil-store fail-closed), resolve (matched-route exchange, miss-denies, non-placeholder passthrough, off-route no-leak), and reverseproxy/extproc inbound propagation (header + body).
• All authlib packages build and pass under -race; all five modules build clean.

Attribution

Assisted-By: Claude Code

Summary by CodeRabbit

• New Features

  • Opt-in credential placeholder swap: mint opaque placeholders for inbound JWTs, store real tokens in a process-scoped TTL store, and resolve placeholders outbound before token exchange.
  • Shared in-memory store wired into proxy, Envoy ext_proc, and lite/proxy entrypoints so mint+resolve work in-process.
  • Inbound propagation now preserves and forwards rewritten Authorization values to backends.

• Tests

  • Added unit, integration, and end-to-end tests validating minting, storage, propagation, resolution, expiry, and concurrent store behavior.

• Documentation

  • Added plugin reference, design/plan docs, and a complete demo README with operational guidance and scripts.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a process-scoped TTL SharedStore, a placeholder handle convention, jwt-validation minting of opaque placeholders, token-exchange resolution, listener and main wiring to propagate Authorization mutations, unit/listener/e2e tests, documentation, and an echo demo.

Changes

Credential Placeholder Swap Feature Implementation

Layer / File(s)	Summary
Shared TTL Store Foundation authbridge/authlib/shared/store.go, authbridge/authlib/shared/store_test.go	Store provides concurrent-safe put/get/delete with TTL, including injectable now for deterministic testing and lazy expiry eviction.
Pipeline Context Extension authbridge/authlib/pipeline/context.go, authbridge/authlib/pipeline/sharedstore_test.go	SharedStore interface and Context.Shared field enable plugins to access the process-scoped store; tests verify interface implementation and field wiring.
Placeholder Handle Convention authbridge/authlib/placeholder/placeholder.go, authbridge/authlib/placeholder/placeholder_test.go	Prefix constant, New() cryptographic handle generation, IsPlaceholder() detection, and Key() namespace derivation standardize opaque credential token handling.
JWT Validation Placeholder Minting authbridge/authlib/plugins/jwtvalidation/plugin.go, authbridge/authlib/plugins/jwtvalidation/placeholder_test.go	placeholder_mode + placeholder_ttl config enable minting opaque handles, storing real tokens in the shared store, and rewriting inbound Authorization headers; mint() helper fails closed when store is unavailable.
Token Exchange Placeholder Resolution authbridge/authlib/plugins/tokenexchange/plugin.go, authbridge/authlib/plugins/tokenexchange/placeholder_test.go	resolve_placeholders config enables looking up real bearer tokens from the shared store before exchanging; resolvePlaceholder() helper denies requests on unresolved/missing handles.
Listener Shared Store Wiring & Authorization Propagation authbridge/authlib/listener/reverseproxy/server.go, authbridge/authlib/listener/reverseproxy/placeholder_test.go, authbridge/authlib/listener/forwardproxy/server.go, authbridge/authlib/listener/extproc/server.go, authbridge/authlib/listener/extproc/placeholder_test.go	Reverse proxy, forward proxy, and ext_proc listeners wire Shared into pipeline contexts; reverse proxy and ext_proc capture/compare pre/post inbound Authorization and emit mutations when changed, propagating minted placeholder tokens to backend/outbound forwarding.
Entry Point Shared Store Integration authbridge/cmd/authbridge-envoy/main.go, authbridge/cmd/authbridge-lite/main.go, authbridge/cmd/authbridge-proxy/main.go	Mains instantiate and wire a shared store into both reverse and forward proxy servers and ext_proc server so listeners in the same process share state.
End-to-end tests & helpers authbridge/authlib/listener/e2e/placeholder_e2e_test.go, test helpers across plugins	E2E verifies inbound minting, shared-store mapping, outbound resolution, and that upstream receives exchanged token while agent/backend never see the real JWT.
Documentation & Design Specifications authbridge/docs/plugin-reference.md, authbridge/docs/superpowers/plans/2026-06-02-credential-placeholder-swap.md, authbridge/docs/superpowers/specs/2026-06-02-credential-placeholder-swap-design.md	Plugin reference documents placeholder_mode/placeholder_ttl and resolve_placeholders; plan and spec describe architecture, configuration, fail-closed semantics, testing, and deployment constraints.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ReverseProxy
  participant JWTValidation
  participant SharedStore
  participant AgentBackend
  participant ForwardProxy
  participant TokenExchange
  participant Upstream

  Client->>ReverseProxy: HTTP request (Authorization: Bearer realJWT)
  ReverseProxy->>JWTValidation: run inbound pipeline (pctx.Shared set)
  JWTValidation->>SharedStore: Put(placeholderKey, realJWT, ttl)  -- mint
  JWTValidation-->>ReverseProxy: mutated Authorization: Bearer abph_<handle>
  ReverseProxy->>AgentBackend: forward request (Authorization: Bearer abph_<handle>)
  AgentBackend->>ForwardProxy: outbound call with Authorization: Bearer abph_<handle>
  ForwardProxy->>TokenExchange: run outbound pipeline (pctx.Shared set)
  TokenExchange->>SharedStore: Get(placeholderKey)  -- resolve
  TokenExchange-->>ForwardProxy: replace Authorization with Bearer EXCHANGED-TOKEN
  ForwardProxy->>Upstream: forward request with exchanged token

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• feat: Placeholder-based credential isolation — agents never see real tokens #336 — matches the placeholder-based credential-isolation flow implemented here (minting, shared store, listener wiring, and resolution).

Suggested reviewers

• cwiklik
• pdettori
• mrsabath

Poem

🐰 I minted a handle, soft and light,

hid a token out of sight,
hopped it to store with tiny care,
swapped it back when routes declare,
now secrets travel safe by night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 32.81% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly and specifically summarizes the main feature: a credential placeholder swap mechanism that hides real tokens from agents using opaque handles. It is concise and directly conveys the primary change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (2)

authbridge/authlib/plugins/tokenexchange/plugin.go (1)

572-578: 💤 Low value

Minor: extract bearer once to avoid duplicate parsing.

auth.ExtractBearer(authHeader) is called twice—once for IsPlaceholder and again for resolvePlaceholder. Extracting once avoids the redundant string parsing.

♻️ Suggested refactor

+	bearer := auth.ExtractBearer(authHeader)
-	if p.cfg.ResolvePlaceholders && placeholder.IsPlaceholder(auth.ExtractBearer(authHeader)) {
-		real, ok := resolvePlaceholder(pctx, auth.ExtractBearer(authHeader))
+	if p.cfg.ResolvePlaceholders && placeholder.IsPlaceholder(bearer) {
+		real, ok := resolvePlaceholder(pctx, bearer)
		if !ok {
			return pctx.DenyAndRecord("placeholder_unresolved", "auth.unauthorized", "unresolvable credential placeholder")
		}
		authHeader = "Bearer " + real
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/authlib/plugins/tokenexchange/plugin.go` around lines 572 - 578,
Extract the bearer token once into a local variable (e.g., token :=
auth.ExtractBearer(authHeader)) before the if; then use that token for
placeholder.IsPlaceholder and resolvePlaceholder calls, and when resolved set
authHeader = "Bearer "+real; update the conditional to check
p.cfg.ResolvePlaceholders && placeholder.IsPlaceholder(token) and pass token to
resolvePlaceholder, keeping pctx.DenyAndRecord unchanged for the failure path.

authbridge/cmd/authbridge-proxy/main.go (1)

255-255: Consider a background eviction sweep for the shared store.

The store created here relies on lazy eviction (entries are only purged on a subsequent Get). Handles minted on the inbound path for passthrough/off-route hosts are never resolved by outbound exchange, so their keys are never Get-ed and never expire — they accumulate for the process lifetime. Under sustained inbound traffic this is unbounded memory growth, not bounded by TTL.

Since this entry point owns the store's lifecycle, consider starting a periodic janitor goroutine (or adding one in authlib/shared) to sweep expired entries, and stopping it during the shutdown sequence. This applies equally to authbridge-lite and authbridge-envoy.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/cmd/authbridge-proxy/main.go` at line 255, The shared store
created with shared.New() uses lazy eviction and can leak unreferenced keys; add
a background janitor goroutine started in main (right after shared.New()) that
periodically calls the store's expiry sweep method (e.g.,
sharedStore.SweepExpired / sharedStore.EvictExpired or a new Sweep method you
add in authlib/shared) on a configurable interval, and ensure the goroutine is
cleanly stopped during the program shutdown sequence (use a context with cancel
or a stop channel tied into your existing shutdown logic in main) so the janitor
terminates before sharedStore is closed; if authlib/shared lacks a sweep API,
implement a Sweep/Prune method there and call it from the janitor.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@authbridge/authlib/listener/extproc/server.go`:
- Around line 164-167: The mint/placeholder response helpers
replaceTokenResponse and replaceTokenBodyResponse currently set the new
Authorization header but do not remove the x-authbridge-direction header,
allowing that direction header to leak; update both functions
(replaceTokenResponse and replaceTokenBodyResponse) to include RemoveHeaders:
[]string{"x-authbridge-direction"} inside the created extprocv3.HeaderMutation
so the direction header is explicitly removed on the mint/placeholder path.

In `@authbridge/authlib/shared/store.go`:
- Around line 30-57: The store currently only evicts expired entries lazily in
Store.Get, which leads to unbounded growth; add a periodic background sweeper
started when the Store is created (e.g. in New) that scans s.items and deletes
entries whose expires <= s.now(), and/or change the resolution path to ensure
consumed entries are deleted (e.g. make Get or the handle-resolution logic call
Store.Delete after successful resolution); ensure the sweeper uses s.mu (Lock)
when mutating s.items and stops/cleans up on Store shutdown to avoid goroutine
leaks (reference Store.Get, Store.Put, Store.Delete and the Store constructor to
locate changes).
- Around line 38-50: Get currently RLocks, releases, then calls Delete which can
remove a concurrently Put value; to fix, after detecting the entry is expired
under the read lock, acquire the write lock and re-check the current map
entry/expiry before deleting. In practice change Store.Get to: read under
mu.RLock as now, and if e exists and appears expired then mu.RUnlock();
mu.Lock(); re-fetch e, ok := s.items[key]; if !ok or s.now().After(e.expires)
then delete and return nil,false; otherwise return e.val,true; ensure you
reference Store.Get, s.mu (RLock/Lock), s.items, s.now(), Delete/Put to locate
the code paths to update.

In `@authbridge/docs/superpowers/plans/2026-06-02-credential-placeholder-swap.md`:
- Line 13: The markdown line currently uses a machine-specific absolute path
string (the text starting with
"/Users/haihuang/works/go/src/github.com/kagenti/kagenti-extensions/authbridge")—replace
that with a repo-relative working-directory instruction instead (for example:
"Run from the authbridge/ repo root" or "All commands run from authbridge/") in
the file 2026-06-02-credential-placeholder-swap.md so contributors and CI have a
portable, non-personalized instruction.

In
`@authbridge/docs/superpowers/specs/2026-06-02-credential-placeholder-swap-design.md`:
- Around line 5-6: Update the top-level "Scope" statement in the spec to
consistently declare the v1 implementation boundary: explicitly list the
components included (jwt-validation plugin, token-exchange plugin, the new
shared store, and the reverseproxy change) and remove or reconcile any mentions
of extproc/extauthz/waypoint that contradict the v1 deferral language; ensure
the same single v1 scope text is used in all locations currently inconsistent
(including the blocks around lines referencing "extauthz/waypoint" and the other
two ranges) and add the behavior note about defaults: AuthBridge defaults to
passthrough outbound policy, token exchange only occurs for hosts in the
authproxy-routes ConfigMap, and set DEFAULT_OUTBOUND_POLICY: "exchange" to
restore legacy behavior.

---

Nitpick comments:
In `@authbridge/authlib/plugins/tokenexchange/plugin.go`:
- Around line 572-578: Extract the bearer token once into a local variable
(e.g., token := auth.ExtractBearer(authHeader)) before the if; then use that
token for placeholder.IsPlaceholder and resolvePlaceholder calls, and when
resolved set authHeader = "Bearer "+real; update the conditional to check
p.cfg.ResolvePlaceholders && placeholder.IsPlaceholder(token) and pass token to
resolvePlaceholder, keeping pctx.DenyAndRecord unchanged for the failure path.

In `@authbridge/cmd/authbridge-proxy/main.go`:
- Line 255: The shared store created with shared.New() uses lazy eviction and
can leak unreferenced keys; add a background janitor goroutine started in main
(right after shared.New()) that periodically calls the store's expiry sweep
method (e.g., sharedStore.SweepExpired / sharedStore.EvictExpired or a new Sweep
method you add in authlib/shared) on a configurable interval, and ensure the
goroutine is cleanly stopped during the program shutdown sequence (use a context
with cancel or a stop channel tied into your existing shutdown logic in main) so
the janitor terminates before sharedStore is closed; if authlib/shared lacks a
sweep API, implement a Sweep/Prune method there and call it from the janitor.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5c67bbb6-ddec-4563-afc7-3cb44ca027c3

📥 Commits

Reviewing files that changed from the base of the PR and between 87b529b and 551b819.

📒 Files selected for processing (21)

• authbridge/authlib/listener/extproc/placeholder_test.go
• authbridge/authlib/listener/extproc/server.go
• authbridge/authlib/listener/forwardproxy/server.go
• authbridge/authlib/listener/reverseproxy/placeholder_test.go
• authbridge/authlib/listener/reverseproxy/server.go
• authbridge/authlib/pipeline/context.go
• authbridge/authlib/pipeline/sharedstore_test.go
• authbridge/authlib/placeholder/placeholder.go
• authbridge/authlib/placeholder/placeholder_test.go
• authbridge/authlib/plugins/jwtvalidation/placeholder_test.go
• authbridge/authlib/plugins/jwtvalidation/plugin.go
• authbridge/authlib/plugins/tokenexchange/placeholder_test.go
• authbridge/authlib/plugins/tokenexchange/plugin.go
• authbridge/authlib/shared/store.go
• authbridge/authlib/shared/store_test.go
• authbridge/cmd/authbridge-envoy/main.go
• authbridge/cmd/authbridge-lite/main.go
• authbridge/cmd/authbridge-proxy/main.go
• authbridge/docs/plugin-reference.md
• authbridge/docs/superpowers/plans/2026-06-02-credential-placeholder-swap.md
• authbridge/docs/superpowers/specs/2026-06-02-credential-placeholder-swap-design.md

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

authbridge/demos/echo/scripts/echo-merge.py (1)

41-81: ⚡ Quick win

Add type hints to merge functions.

The coding guidelines require Python 3.12+ syntax with type hints. While main() has a return type hint, merge_inbound and merge_outbound are missing parameter and return type hints.

✏️ Proposed type hints

-def merge_inbound(plugins, jwt_cfg):
+def merge_inbound(plugins: list[dict], jwt_cfg: dict) -> None:
     """Set placeholder_mode / placeholder_ttl on every jwt-validation
     plugin entry's config. Creates the config map if missing."""

-def merge_outbound(plugins, te_cfg, routes):
+def merge_outbound(plugins: list[dict], te_cfg: dict, routes: list[dict]) -> None:
     """Set resolve_placeholders on every token-exchange plugin entry and
     append the patch's routes into config.routes.rules (by `host`)."""

As per coding guidelines: "Use Python 3.12+ syntax, including type hints with str | None notation"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/demos/echo/scripts/echo-merge.py` around lines 41 - 81, Add Python
3.12+ type hints to both functions: change merge_inbound signature to accept
plugins: list[dict[str, object]] and jwt_cfg: dict[str, object] and return None,
and change merge_outbound signature to accept plugins: list[dict[str, object]],
te_cfg: dict[str, object], routes: list[dict[str, object]] and return None; use
built-in generic types (PEP 585) and Python 3.12 union syntax where applicable
so callers of merge_inbound and merge_outbound (look for those exact function
names) get explicit parameter/return typing.

authbridge/demos/echo/agent/main.go (1)

1-398: 💤 Low value

Consider using log/slog for consistency.

The coding guidelines specify using log/slog for logging throughout authbridge binaries. This demo agent uses the standard log package. While functional, switching to slog would maintain consistency with the rest of the authbridge codebase and provide structured logging benefits.

As per coding guidelines: "Use log/slog for logging throughout authlib and authbridge binaries; binaries log under their own name (authbridge-proxy, authbridge-envoy, authbridge-lite)"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/demos/echo/agent/main.go` around lines 1 - 398, Replace use of the
standard log package with log/slog: add import "log/slog", add a package-level
logger (e.g., logger := slog.New(slog.NewTextHandler(os.Stderr))) and update all
logging calls (log.Printf, log.Fatalf, log.Printf in
buildProxiedClient/handleAgentCard/handleA2A/runEcho/callUpstreamEcho/main/writeRPCSuccess/writeRPCError/newUUID)
to use logger.Info or logger.Error (passing context.Background() as the first
arg) and where log.Fatalf was used, call logger.Error(...) then os.Exit(1) to
preserve exit behavior.

authbridge/demos/echo/scripts/setup_keycloak.py (1)

100-193: 💤 Low value

Add type hints to helper functions.

Per coding guidelines, Python code in authbridge/ must use Python 3.12 syntax with type hints. Only get_spiffe_id has annotations; the remaining helpers should follow suit.

♻️ Example type annotations

-def get_or_create_realm(keycloak_admin, realm_name):
+def get_or_create_realm(keycloak_admin: KeycloakAdmin, realm_name: str) -> None:

-def get_or_create_client(keycloak_admin, client_payload):
+def get_or_create_client(keycloak_admin: KeycloakAdmin, client_payload: dict) -> str | None:

-def get_or_create_client_scope(keycloak_admin, scope_payload):
+def get_or_create_client_scope(keycloak_admin: KeycloakAdmin, scope_payload: dict) -> str:

-def add_audience_mapper(keycloak_admin, scope_id, mapper_name, audience):
+def add_audience_mapper(keycloak_admin: KeycloakAdmin, scope_id: str, mapper_name: str, audience: str) -> None:

-def get_or_create_user(keycloak_admin, user_config):
+def get_or_create_user(keycloak_admin: KeycloakAdmin, user_config: dict) -> str:

As per coding guidelines: "Python code must use Python 3.12 syntax with type hints".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/demos/echo/scripts/setup_keycloak.py` around lines 100 - 193, Add
Python 3.12 type hints to the helper functions: annotate parameters and return
types for get_or_create_realm(keycloak_admin, realm_name) -> None,
get_or_create_client(keycloak_admin, client_payload: dict[str, Any]) -> str,
get_or_create_client_scope(keycloak_admin, scope_payload: dict[str, Any]) ->
str, add_audience_mapper(keycloak_admin, scope_id: str, mapper_name: str,
audience: str) -> None, and get_or_create_user(keycloak_admin, user_config:
dict[str, Any]) -> str; also add the necessary typing imports (e.g., from typing
import Any) and use Python 3.12 union syntax where appropriate if you choose to
allow Optional returns. Ensure the function signatures in this file match these
annotations and update any docstrings/comments if needed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@authbridge/demos/echo/agent/main.go`:
- Line 1: The go.work file is missing the demo module, so CodeQL can't analyze
authbridge/demos/echo; open authbridge/go.work and add the demo module to the
use(...) block (for example add a line like use ./demos/echo) so the workspace
includes the demo's go.mod and then re-run CodeQL extraction; target the go.work
file and ensure the new entry covers the module for the authbridge/demos/echo
demo.

---

Nitpick comments:
In `@authbridge/demos/echo/agent/main.go`:
- Around line 1-398: Replace use of the standard log package with log/slog: add
import "log/slog", add a package-level logger (e.g., logger :=
slog.New(slog.NewTextHandler(os.Stderr))) and update all logging calls
(log.Printf, log.Fatalf, log.Printf in
buildProxiedClient/handleAgentCard/handleA2A/runEcho/callUpstreamEcho/main/writeRPCSuccess/writeRPCError/newUUID)
to use logger.Info or logger.Error (passing context.Background() as the first
arg) and where log.Fatalf was used, call logger.Error(...) then os.Exit(1) to
preserve exit behavior.

In `@authbridge/demos/echo/scripts/echo-merge.py`:
- Around line 41-81: Add Python 3.12+ type hints to both functions: change
merge_inbound signature to accept plugins: list[dict[str, object]] and jwt_cfg:
dict[str, object] and return None, and change merge_outbound signature to accept
plugins: list[dict[str, object]], te_cfg: dict[str, object], routes:
list[dict[str, object]] and return None; use built-in generic types (PEP 585)
and Python 3.12 union syntax where applicable so callers of merge_inbound and
merge_outbound (look for those exact function names) get explicit
parameter/return typing.

In `@authbridge/demos/echo/scripts/setup_keycloak.py`:
- Around line 100-193: Add Python 3.12 type hints to the helper functions:
annotate parameters and return types for get_or_create_realm(keycloak_admin,
realm_name) -> None, get_or_create_client(keycloak_admin, client_payload:
dict[str, Any]) -> str, get_or_create_client_scope(keycloak_admin,
scope_payload: dict[str, Any]) -> str, add_audience_mapper(keycloak_admin,
scope_id: str, mapper_name: str, audience: str) -> None, and
get_or_create_user(keycloak_admin, user_config: dict[str, Any]) -> str; also add
the necessary typing imports (e.g., from typing import Any) and use Python 3.12
union syntax where appropriate if you choose to allow Optional returns. Ensure
the function signatures in this file match these annotations and update any
docstrings/comments if needed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0053c2cb-d54e-4497-95c9-bbf5ff90ffa7

📥 Commits

Reviewing files that changed from the base of the PR and between f7ab1c3 and a3aa80c.

📒 Files selected for processing (14)

• authbridge/demos/echo/.gitignore
• authbridge/demos/echo/Makefile
• authbridge/demos/echo/README.md
• authbridge/demos/echo/agent/Dockerfile
• authbridge/demos/echo/agent/main.go
• authbridge/demos/echo/go.mod
• authbridge/demos/echo/k8s/agent.yaml
• authbridge/demos/echo/k8s/echo-patch.yaml
• authbridge/demos/echo/k8s/upstream.yaml
• authbridge/demos/echo/scripts/echo-merge.py
• authbridge/demos/echo/scripts/patch-echo-config.sh
• authbridge/demos/echo/scripts/setup_keycloak.py
• authbridge/demos/echo/upstream/Dockerfile
• authbridge/demos/echo/upstream/main.go

✅ Files skipped from review due to trivial changes (5)

• authbridge/demos/echo/.gitignore
• authbridge/demos/echo/go.mod
• authbridge/demos/echo/k8s/echo-patch.yaml
• authbridge/demos/echo/agent/Dockerfile
• authbridge/demos/echo/k8s/upstream.yaml

[pdettori]

Excellent security-critical feature with strong design principles: fail-closed error handling, real tokens never forwarded to agents, 256-bit CSPRNG placeholders, proper TOCTOU mitigation in the store, and comprehensive E2E + unit test coverage.

All 21 commits signed-off. CI fully green. No secrets in production code. Config fields consistently named across all files.

Areas reviewed: Go (auth libraries, plugins, listeners, main binaries), Tests (unit + E2E), Shell, YAML, Docs, Demo

Assisted-By: Claude Code

[pdettori]

nit: Consider accepting a context.Context in New() so the janitor goroutine respects context-based cancellation (idiomatic for Go services with graceful shutdown). The current channel-based Close() works correctly but diverges from the context pattern used elsewhere in the pipeline.

[pdettori]

suggestion: The concurrency test uses 50 goroutines writing different keys (mod 26). Adding a focused test that hammers Put+Get on the same key concurrently would further strengthen the TOCTOU protection assertion in Get()'s double-check eviction logic.