feat(hub): POST /api/v1/agent/secrets — the fetch endpoint (#127, P2c) - #1340
Open
ptone wants to merge 7 commits into
Open
feat(hub): POST /api/v1/agent/secrets — the fetch endpoint (#127, P2c)#1340ptone wants to merge 7 commits into
ptone wants to merge 7 commits into
Conversation
added 7 commits
August 29, 2026 01:03
The agent secret-fetch endpoint. Two gates, both required:
Gate 1 (stored list): credential row's EntitledSecretKeys by JTI hash.
Cheap, blocks enumeration, session-stable.
Gate 2 (live resolution): scope filter + progeny authz at request time.
Makes revocation take effect immediately.
Five outcomes per key:
1. In stored list, resolves now → the value
2. In stored list, exists but unreadable → "entitled but unavailable"
3. In stored list, no longer authorized → "access withdrawn, refresh"
4. Not in stored list → indistinguishable from "does not exist"
5. No credential row → "token predates entitlement recording"
Three column states (NULL, empty, populated) each handled distinctly.
NULL fails closed loudly — indicates a minting-path bug.
New scope ScopeAgentSecretFetch ("agent:secret:fetch") granted to
baseline and full roles. It is a ROLE FILTER, not a capability grant.
Smart scope guard distinguishes pre-existing tokens (role would receive
scope but token doesn't carry it) from genuinely unprivileged tokens.
The middleware fails open on credential-store errors (auth.go:179-183).
This handler does its own lookup and fails closed. The double fetch is
accepted and load-bearing.
…P2c) 14 tests covering the full P2c requirement set: Five outcomes (each with a named test): - Row 1: happy path — entitled, resolves, returns value - Row 2: entitled but unavailable — encrypted value, no key - Row 3: access withdrawn — secret de-scoped after credential mint - Row 4: not in stored list — indistinguishable from "does not exist" - Row 5: no credential row — legacy pre-table token Three column states: - NULL: minting-path bug, fails closed loudly - Empty list: entitled to nothing, all keys are row 4 - Populated: normal operation, mixed match/no-match Smart scope guard (two branches): - Pre-existing token: role WOULD receive scope, says "refresh" - Genuine denial: role would NOT receive scope, generic message Gate-1 mutation test: entitled key returned, non-entitled key blocked, proving gate 1 is load-bearing. Scope-present-but-empty-entitlement: scope IS present, entitled list empty, fetch denied. Proves the scope is not the control. Also updates ScopesForRole tests for new ScopeAgentSecretFetch (baseline 6 scopes, full 9 scopes).
The requireAgentSecretFetchScope guard uses ScopeAgentStatusUpdate as a proxy for "would this token's role receive ScopeAgentSecretFetch" because AgentTokenClaims carries scopes but not the role string — the role is consumed at mint-time and discarded. Add a full comment explaining WHY the proxy is forced (no Role field in JWT claims) and WHAT BREAKS if ScopeAgentStatusUpdate's role assignment changes independently. Add TestScopeGuardProxy_DriftDetection: iterates all four roles and asserts the biconditional — a role receives ScopeAgentSecretFetch iff it receives ScopeAgentStatusUpdate. If a future agentrole.go change breaks this coupling, the test fails in the file being edited, not silently in an error message during token migration.
No canonical AllRoles() exists — ValidAgentRole is a switch. The drift test now asserts the known role count (4) and validates every entry against ValidAgentRole. A fifth role added to ValidAgentRole without updating the test triggers a failure with an actionable message pointing to the biconditional and requireAgentSecretFetchScope. Also probes a broader set of plausible role names as a second line of defense against roles that bypass the count check.
Add AllAgentRoles() as the single source of truth for stock roles. Rewrite ValidAgentRole to iterate it, so the two cannot drift apart. A fifth role must be added to AllAgentRoles() to be valid at all, which means the drift-detection test covers it automatically. Replace the three-layer tripwire (circular count check, validation loop, candidate probe) with a single iteration of AllAgentRoles(). The test is now just the biconditional assertion it was always meant to be. Behaviour is identical: ValidAgentRole accepts exactly the same four values and rejects everything else. All existing role tests pass untouched.
R1: Hoist computeEntitledSecretKeys out of the per-key loop. One listing per request, not one per key. The state that triggers the entitled-but-not-resolved branch is correlated — a backend wobble or key rotation puts many keys into it at once — so the multi-key case is the common case, not the rare one. O1 (elevated to Required): When listErr is non-nil, default to the transient reading (row 2 / unavailable), not the permanent one (row 3 / access withdrawn). The old code fell through to row 3 on listing errors, telling the operator "access has been withdrawn; refresh the token" when nothing was revoked and refreshing cannot fix a backend outage. An operator who believes entitlement was withdrawn may re-grant permissions that were never lost — a security-relevant change made on a false report. R2: PR description updated to state "This does not close #127. P3 is the phase that stops the leak."
Add TestSecretFetch_ListingError_ReportsUnavailableNotWithdrawn: when the entitlement listing fails (backend outage), the handler must report "unavailable" not "access withdrawn". Asserts status == unavailable, error mentions "listing", and error does NOT contain "withdrawn" or "refresh". Mutation verified: deleting the `if listErr != nil` branch causes the test to go red with four failures — status flips to access_withdrawn, error loses "listing", gains "withdrawn" and "refresh". Restored, green. Seam: failingListSecretsStore wraps the real store and overrides ListSecrets to return an error. The secret backend's internal store remains real, so Resolve() succeeds. The handler's computeEntitledSecretKeys uses s.store (the wrapper), which fails.
This was referenced Aug 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The agent secret-fetch endpoint — the channel that makes P3 (removing credentials from argv) possible.
This does not close #127. P3 is the phase that stops the leak.
POST /api/v1/agent/secrets— authenticated agents fetch secret values by key nameScopeAgentSecretFetchgranted to baseline and full rolesDesign decisions
Both gates, not either (§3.6.2): Gate 1 is the stored entitled-key list from the credential row, looked up by JTI hash. Gate 2 is live resolution with scope filter + progeny authz. The answer is the intersection. This keeps the stored list load-bearing without making it authoritative.
Five outcomes: (1) value returned, (2) entitled but unavailable (decryption failure), (3) access withdrawn since mint, (4) not in stored list (indistinguishable from "does not exist"), (5) no credential row (legacy token).
Three column states: populated list (normal), empty list (entitled to nothing), NULL (minting-path bug — fails closed loudly). All three tested explicitly.
Smart scope guard: Tokens whose role WOULD receive the scope but don't carry it (pre-existing) get "issued before secret-fetch capability existed; restart or refresh." Tokens whose role would NOT receive it get generic "insufficient scope." Both branches tested. Wrong advice (telling a readonly agent to refresh) would be worse than no advice.
Double fetch accepted: Auth middleware fetches the credential row and keeps only
cred.ID. This handler re-fetches by JTI hash to getEntitledSecretKeys. This is load-bearing — the middleware fails open on ErrNotFound and store errors; this handler fails closed.Credential-minting path enumeration
Five paths at base 8840155, searched
git grep -n 'GenerateAgentToken\b' 8840155 -- 'pkg/hub/*.go':httpdispatcher.go:452— DispatchAgentCreatehttpdispatcher.go:1983— DispatchAgentStarthttpdispatcher.go:2242— DispatchAgentRestarthttpdispatcher.go:2358— DispatchAgentResetAuthhandlers_agents_core.go:2789— handleAgentTokenRefreshAll five record entitled keys. The sixth (RefreshAgentToken) was deleted in PR #1338.
Test plan
14 tests, all passing:
TestSecretFetch_Row1_HappyPath— entitled, resolves, returns valueTestSecretFetch_Row2_EntitledButUnavailable— encrypted value, no keyTestSecretFetch_Row3_AccessWithdrawn— revocation-rejection testTestSecretFetch_Row4_NotInStoredList— indistinguishable from "not found"TestSecretFetch_Row5_NoCredentialRow— legacy pre-table tokenTestSecretFetch_ColumnNULL— minting-path bug detectionTestSecretFetch_ColumnEmptyList— entitled to nothingTestSecretFetch_ColumnPopulated— normal mixed operationTestSecretFetch_ScopeGuard_PreExistingToken— "refresh" adviceTestSecretFetch_ScopeGuard_GenuineDenial— generic denial, no refresh adviceTestSecretFetch_ScopePresentButEntitlementEmpty— scope ≠ controlTestSecretFetch_Gate1Mutation— gate 1 is load-bearingTestSecretFetch_NoBody— 400 on missing keysTestSecretFetch_MethodNotAllowed— 405 on GETgo vet ./pkg/hub/— cleango build ./...— cleanTestScopesForRole_Baseline/_Full— updated for new scope count