You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up findings from the zcode (GLM-5.3) review of #1389 that were below the merge bar (no critical/high). Each was verified against the code; fix in small PRs.
low — internal/storage/client_credentials.go:190-192: StageClientCredentialRotation returns ErrClientCredentialActive ("client credential is active; use rotation instead of mint") when the target record is actually revoked/expired — the inverse of what the sentinel text asserts and the wrong classification for a caller that branches on it to decide mint-vs-rotate remediation.
low — internal/storage/client_credentials.go:391-408: ForgetClientCredential is the one client-credential mutation that is not a single bbolt transaction: it reads the record via GetAgentTokenByName (a separate RLock/View) and revokes by name via RevokeAgentToken (a separate Lock/Update, re-resolved by name). A concurrent MintClientCredential replacing an expiring record between the two locks makes the second call revoke the newer record while the function returns the caller stale TokenHash/TokenPrefix from the record it originally inspected.
low — internal/storage/client_credentials_test.go:67-93: T028's claim 'mint replaces an existing revoked OR expired kind=client record' is only tested for the revoked variant (TestMintClientCredential_ReplacesRevokedOrExpired revokes, never lets a record actually expire before re-minting).
low — internal/server/session_store.go:449-483: FR-028 ('sessions MUST record the authenticating token name and client id at initialize') has SetSessionIdentity/UpdateSessionProfile/SessionsForToken implemented and unit-tested, but zero production callers anywhere in the repo — nothing stamps TokenName/ClientID at session initialize. The commit message names only the 'session→server-instance map' as deferred, which reads as though identity-stamping itself landed; tasks.md's own ledger is more honest (T039 is left unchecked). Downgraded from zcode's 'medium': this is staged-PR practice consistent with the rest of this PR (ResolveProfileV3 is likewise unwired and explicitly not to be treated as a defect), not a new inconsistency, so I file it as a low completeness note rather than a functional gap.
Follow-up findings from the zcode (GLM-5.3) review of #1389 that were below the merge bar (no critical/high). Each was verified against the code; fix in small PRs.
internal/storage/client_credentials.go:190-192: StageClientCredentialRotation returns ErrClientCredentialActive ("client credential is active; use rotation instead of mint") when the target record is actually revoked/expired — the inverse of what the sentinel text asserts and the wrong classification for a caller that branches on it to decide mint-vs-rotate remediation.internal/storage/client_credentials.go:391-408: ForgetClientCredential is the one client-credential mutation that is not a single bbolt transaction: it reads the record via GetAgentTokenByName (a separate RLock/View) and revokes by name via RevokeAgentToken (a separate Lock/Update, re-resolved by name). A concurrent MintClientCredential replacing an expiring record between the two locks makes the second call revoke the newer record while the function returns the caller stale TokenHash/TokenPrefix from the record it originally inspected.internal/storage/client_credentials_test.go:67-93: T028's claim 'mint replaces an existing revoked OR expired kind=client record' is only tested for the revoked variant (TestMintClientCredential_ReplacesRevokedOrExpired revokes, never lets a record actually expire before re-minting).internal/server/session_store.go:449-483: FR-028 ('sessions MUST record the authenticating token name and client id at initialize') has SetSessionIdentity/UpdateSessionProfile/SessionsForToken implemented and unit-tested, but zero production callers anywhere in the repo — nothing stamps TokenName/ClientID at session initialize. The commit message names only the 'session→server-instance map' as deferred, which reads as though identity-stamping itself landed; tasks.md's own ledger is more honest (T039 is left unchecked). Downgraded from zcode's 'medium': this is staged-PR practice consistent with the rest of this PR (ResolveProfileV3 is likewise unwired and explicitly not to be treated as a defect), not a new inconsistency, so I file it as a low completeness note rather than a functional gap.