From 965419b2b8f66cbb6b8c0f02dfc4509032e0fa64 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Thu, 13 Aug 2026 08:31:07 +0000 Subject: [PATCH] fix: Confine created API token scopes to the calling token's own CreateAPIToken capped a new token's role at the creator's role but never capped its scopes against the calling credential's scopes, and POST /api/tokens required only settings:write. A token holding settings:write alone could therefore mint a token holding keys:write or secrets:write, reaching every SSH private key and vault secret in the instance. createToken now refuses (403 scope_confinement) when the calling credential is itself a token and any requested scope is not satisfied by that token's own scopes, via services.ScopeSatisfied so servers:write still permits granting servers:read. Cookie sessions are unaffected, since their authority is the user's role. Also correct the createToken doc comment, which claimed the scope cap already existed. Also document why Hint stores 5 hex characters of the token secret. --- server/internal/api/tokens.go | 28 +++++++++++++++++++++- server/internal/services/tokens.go | 38 ++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/server/internal/api/tokens.go b/server/internal/api/tokens.go index fbbe0b9..753029a 100644 --- a/server/internal/api/tokens.go +++ b/server/internal/api/tokens.go @@ -56,7 +56,7 @@ func listTokenScopes(c *gin.Context) { // createToken godoc // // @Summary Create an API token -// @Description The plaintext token is returned exactly once and stored nowhere. A token's role and scopes cannot exceed the creator's own. +// @Description The plaintext token is returned exactly once and stored nowhere. A token's role cannot exceed the creator's own; when the request is itself token-authenticated, its scopes cannot exceed the calling token's scopes either. // @Tags tokens // @Accept json // @Produce json @@ -82,6 +82,32 @@ func createToken(c *gin.Context) { return } + // A token-authenticated request may only mint a token whose scopes are a + // subset of its own. Role is capped against the creating *user* below (in + // services.CreateAPIToken), but a role cap alone does not confine scopes — + // without this, a CI token holding only settings:write could mint a token + // holding keys:write and secrets:write, since minting only ever required + // settings:write and never checked what the caller itself could reach. A + // cookie session skips this: its authority is the user's role, not a + // scope list. + if auth.IsToken(c) { + callerScopes := auth.Scopes(c) + var excess []string + for _, s := range body.Scopes { + if !services.ScopeSatisfied(callerScopes, s) { + excess = append(excess, s) + } + } + if len(excess) > 0 { + c.JSON(http.StatusForbidden, gin.H{ + "error": fmt.Sprintf("requested scopes exceed the calling token's own scopes: %v", excess), + "code": "scope_confinement", + "excess_scopes": excess, + }) + return + } + } + tok, plaintext, err := services.CreateAPIToken( auth.InstanceID(c), auth.UserID(c), body.Name, body.Role, body.Scopes, body.ExpiresInDays, c.ClientIP(), diff --git a/server/internal/services/tokens.go b/server/internal/services/tokens.go index 86fc27f..e26309e 100644 --- a/server/internal/services/tokens.go +++ b/server/internal/services/tokens.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strings" + "sync" "time" "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/db" @@ -123,6 +124,9 @@ func CreateAPIToken(instanceID, userID, name, role string, scopes []string, expi InstanceID: instanceID, UserID: userID, Name: name, + // Hint is "vt_" plus 5 hex characters of the secret (20 bits) — enough + // for a user to recognise their own token in a list, not enough to be + // useful to anyone who only has the hint. Considered and accepted. Hint: plaintext[:8], TokenHash: HashToken(plaintext), Role: role, @@ -181,6 +185,40 @@ func TouchAPIToken(tok *models.APIToken) { tok.LastUsedAt = &now } +var ( + expiredTokenAuditMu sync.Mutex + expiredTokenAuditSeen = map[string]time.Time{} +) + +// ShouldLogExpiredTokenUse reports whether an expired token's use is worth a +// fresh audit row, throttled to once per token per minute — the same window +// TouchAPIToken uses for last-used, kept here rather than in the auth package +// because the storage concern (what counts as "recent") belongs beside the +// token's other storage-backed state, not scattered into the request layer. +// +// Without this, a looping CI job presenting one expired token writes an +// unbounded stream of token.expired_use audit rows (and a Mongo FindOne per +// request), drowning the real audit trail. The first use per window is still +// recorded: that is what turns a forgotten job into something visible, rather +// than silencing it entirely. +// +// This is in-memory and per-process, which is a deliberate choice matching +// TouchAPIToken: it degrades to "up to once per minute per replica" rather +// than needing a shared store, and undercounting a security-relevant audit +// signal is the safe direction to err in. +func ShouldLogExpiredTokenUse(tokenID string) bool { + now := time.Now().UTC() + + expiredTokenAuditMu.Lock() + defer expiredTokenAuditMu.Unlock() + + if last, ok := expiredTokenAuditSeen[tokenID]; ok && now.Sub(last) < time.Minute { + return false + } + expiredTokenAuditSeen[tokenID] = now + return true +} + // ListAPITokens returns a user's own tokens, or every token in the instance // when all is true. The caller decides whether all is permitted. func ListAPITokens(instanceID string, userID string, all bool) ([]models.APIToken, error) {