InstaNode-dev
diff --git a/‎internal/handlers/agent_action_name_too_long_test.go‎
Lines changed: 54 additions & 0 deletions b/‎internal/handlers/agent_action_name_too_long_test.go‎
Lines changed: 54 additions & 0 deletions
diff --git a/‎internal/handlers/auth_logout.go‎
Lines changed: 10 additions & 5 deletions b/‎internal/handlers/auth_logout.go‎
Lines changed: 10 additions & 5 deletions
diff --git a/‎internal/handlers/auth_logout_coverage_test.go‎
Lines changed: 31 additions & 9 deletions b/‎internal/handlers/auth_logout_coverage_test.go‎
Lines changed: 31 additions & 9 deletions
diff --git a/‎internal/handlers/helpers.go‎
Lines changed: 6 additions & 1 deletion b/‎internal/handlers/helpers.go‎
Lines changed: 6 additions & 1 deletion
diff --git a/‎internal/handlers/magic_link.go‎
Lines changed: 10 additions & 1 deletion b/‎internal/handlers/magic_link.go‎
Lines changed: 10 additions & 1 deletion
diff --git a/‎internal/handlers/magic_link_token_alias_test.go‎
Lines changed: 29 additions & 0 deletions b/‎internal/handlers/magic_link_token_alias_test.go‎
Lines changed: 29 additions & 0 deletions
diff --git a/‎internal/middleware/auth.go‎
Lines changed: 64 additions & 7 deletions b/‎internal/middleware/auth.go‎
Lines changed: 64 additions & 7 deletions
@@ -0,0 +1,54 @@
+package handlers
+
+// agent_action_name_too_long_test.go — BUG-AUTH-006 regression.
+//
+// The `name_too_long` agent_action sentence used to say "exceeds 64
+// characters" and "Shorten it to a short human label (1-64 chars)" — but
+// the cap varies per endpoint:
+//
+//	- resource names      : 1-64 chars
+//	- PAT (API key) names : 1-120 chars
+//	- team names          : 1-200 chars
+//
+// PATs hit 120 → message reads "must be 120 characters or fewer" →
+// agent_action contradicts message → agent renders contradiction to user.
+// Fix: keep agent_action cap-free; tell the agent to read the
+// endpoint-specific limit from `message`.
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAgentAction_NameTooLong_DoesNotBakeCap(t *testing.T) {
+	src, err := os.ReadFile("helpers.go")
+	require.NoError(t, err, "helpers.go must be readable from package dir")
+	body := string(src)
+
+	// Scope the assertion to just the name_too_long registry entry — not
+	// other agent_action sentences in the same file.
+	idx := strings.Index(body, `"name_too_long":`)
+	require.GreaterOrEqual(t, idx, 0, "name_too_long entry not found in registry")
+	end := idx + 600
+	if end > len(body) {
+		end = len(body)
+	}
+	window := body[idx:end]
+
+	// BUG-AUTH-006: the sentence must NOT advertise "exceeds 64" or
+	// "(1-64 chars)" — those contradict the actual handler caps for PAT
+	// names (120) and team names (200).
+	assert.NotContains(t, window, "exceeds 64 characters",
+		"BUG-AUTH-006: agent_action must not bake the 64-char cap into the sentence")
+	assert.NotContains(t, window, "(1-64 chars)",
+		"BUG-AUTH-006: agent_action must not bake the 64-char range into the sentence")
+
+	// Positive: the new sentence MUST tell the agent to read the
+	// per-endpoint cap from the `message` field.
+	assert.Contains(t, window, "message",
+		"BUG-AUTH-006: agent_action must direct the agent to read the limit from `message`")
+}
@@ -94,12 +94,16 @@ func NewLogoutHandler(cfg *config.Config, rdb *redis.Client) *LogoutHandler {
 func (h *LogoutHandler) Logout(c *fiber.Ctx) error {
 	requestID := middleware.GetRequestID(c)
 
-	// RequireAuth already validated the token — but we need the raw JWT to
-	// extract the jti and exp claims. Re-parse without secret validation is
-	// wrong; re-parse with the secret is the correct approach.
+	// BUG-AUTH-005: /auth/logout is idempotent per the OpenAPI contract
+	// ("safe to call without a valid token"). Missing / malformed /
+	// expired credentials therefore return 200 {ok:true} with no
+	// revocation work — the local token is already useless, so the
+	// dashboard's logout-on-expiry path must not surface a confusing 401
+	// here. Tokens that DO parse cleanly carry a jti to revoke (the path
+	// below handles them).
 	header := c.Get("Authorization")
 	if len(header) < 8 || !strings.EqualFold(header[:7], "Bearer ") {
-		return respondError(c, fiber.StatusUnauthorized, "unauthorized", "Authorization header required")
+		return c.JSON(fiber.Map{"ok": true})
 	}
 	tokenStr := header[7:]
 
@@ -117,7 +121,8 @@ func (h *LogoutHandler) Logout(c *fiber.Ctx) error {
 		return []byte(h.cfg.JWTSecret), nil
 	}, jwt.WithValidMethods([]string{"HS256"}))
 	if err != nil || !parsed.Valid {
-		return respondError(c, fiber.StatusUnauthorized, "unauthorized", "Token invalid or expired")
+		// BUG-AUTH-005: idempotent — token won't parse, nothing to revoke.
+		return c.JSON(fiber.Map{"ok": true})
 	}
 
 	jti := claims.ID
 
@@ -72,6 +72,11 @@ func mintLogoutJWT(t *testing.T, jti string, expDelta time.Duration) string {
 	return s
 }
 
+// BUG-AUTH-005: /auth/logout is idempotent per the OpenAPI contract —
+// "safe to call without a valid token." Missing / non-Bearer / wrong-secret
+// credentials must return 200 {ok:true} (the local token is already
+// useless, so the dashboard's logout-on-expiry path can't surface a
+// confusing 401). Pre-fix all three returned 401.
 func TestLogout_MissingAuthorizationHeader(t *testing.T) {
 	rdb, clean := setupCoverageRedis(t)
 	defer clean()
@@ -81,7 +86,7 @@ func TestLogout_MissingAuthorizationHeader(t *testing.T) {
 	resp, err := app.Test(req, 5000)
 	require.NoError(t, err)
 	defer resp.Body.Close()
-	assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
+	assert.Equal(t, http.StatusOK, resp.StatusCode, "BUG-AUTH-005: idempotent on missing auth")
 }
 
 func TestLogout_NonBearerAuthorization(t *testing.T) {
@@ -94,7 +99,7 @@ func TestLogout_NonBearerAuthorization(t *testing.T) {
 	resp, err := app.Test(req, 5000)
 	require.NoError(t, err)
 	defer resp.Body.Close()
-	assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
+	assert.Equal(t, http.StatusOK, resp.StatusCode, "BUG-AUTH-005: idempotent on non-Bearer scheme")
 }
 
 func TestLogout_WrongSecretJWT(t *testing.T) {
@@ -117,7 +122,7 @@ func TestLogout_WrongSecretJWT(t *testing.T) {
 	resp, err := app.Test(req, 5000)
 	require.NoError(t, err)
 	defer resp.Body.Close()
-	assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
+	assert.Equal(t, http.StatusOK, resp.StatusCode, "BUG-AUTH-005: idempotent on wrong-secret JWT (parse fail)")
 }
 
 func TestLogout_NoJTIIsNoOp(t *testing.T) {
@@ -165,19 +170,24 @@ func TestLogout_HappyPath_WritesRedisRevocationKey(t *testing.T) {
 	assert.LessOrEqual(t, ttl, 2*time.Hour+time.Second)
 }
 
-func TestLogout_ExpiredButValidlySignedTokenStillRejectedByJWTParse(t *testing.T) {
+// BUG-AUTH-005: an expired-but-validly-signed token is a no-op. The
+// underlying jwt library refuses to parse it (exp in the past), so the
+// handler treats it as "nothing to revoke" and returns 200 {ok:true}.
+// Pre-fix this returned 401 — which broke the dashboard's
+// logout-on-expiry path because the local token was already useless.
+func TestLogout_ExpiredButValidlySignedTokenIsIdempotent(t *testing.T) {
 	rdb, clean := setupCoverageRedis(t)
 	defer clean()
 	app := newLogoutApp(t, rdb)
 
-	// jwt library rejects expired tokens; the handler returns 401.
 	tokenStr := mintLogoutJWT(t, uuid.New().String(), -time.Minute)
 	req := httptest.NewRequest(http.MethodPost, "/auth/logout", nil)
 	req.Header.Set("Authorization", "Bearer "+tokenStr)
 	resp, err := app.Test(req, 5000)
 	require.NoError(t, err)
 	defer resp.Body.Close()
-	assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
+	assert.Equal(t, http.StatusOK, resp.StatusCode,
+		"BUG-AUTH-005: expired token = no-op, returns 200 (idempotent)")
 }
 
 func TestLogout_RedisFailureReturns503(t *testing.T) {
@@ -235,7 +245,12 @@ func TestLogout_TokenWithoutExpDefaultsTo24h(t *testing.T) {
 	assert.LessOrEqual(t, ttl, 24*time.Hour+time.Second)
 }
 
-func TestLogout_HS384RejectedAsUnexpectedSigningMethod(t *testing.T) {
+// BUG-AUTH-005 + T10 P2-1: an HS384-signed token still fails the
+// jwt.WithValidMethods(["HS256"]) gate at parse-time, so the handler
+// treats it as "nothing to revoke" and returns 200 (idempotent contract).
+// The alg pin is still enforced — a wrong-alg JWT never lands in the
+// revocation set. Pre-AUTH-005 fix this returned 401.
+func TestLogout_HS384TokenIsIdempotent(t *testing.T) {
 	rdb, clean := setupCoverageRedis(t)
 	defer clean()
 	app := newLogoutApp(t, rdb)
@@ -255,6 +270,13 @@ func TestLogout_HS384RejectedAsUnexpectedSigningMethod(t *testing.T) {
 	resp, err := app.Test(req, 5000)
 	require.NoError(t, err)
 	defer resp.Body.Close()
-	assert.Equal(t, http.StatusUnauthorized, resp.StatusCode,
-		"HS384 must be rejected; only HS256 is accepted")
+	assert.Equal(t, http.StatusOK, resp.StatusCode,
+		"BUG-AUTH-005: HS384 won't parse → idempotent 200, jti never revoked")
+
+	// Verify the alg-pin still bars the jti from landing in Redis.
+	key := RevokedJTIKey(rc.ID)
+	exists, err := rdb.Exists(context.Background(), key).Result()
+	require.NoError(t, err)
+	assert.Equal(t, int64(0), exists,
+		"HS384 token must NOT result in a Redis revocation row (alg pin honoured)")
 }
@@ -441,7 +441,12 @@ var codeToAgentAction = map[string]errorCodeMeta{
 		AgentAction: "Tell the user the confirm_slug field is required to confirm this destructive action — supply the slug exactly as shown in the prompt and retry — see https://instanode.dev/docs.",
 	},
 	"name_too_long": {
-		AgentAction: "Tell the user the 'name' field exceeds 64 characters. Shorten it to a short human label (1-64 chars) and retry — see https://instanode.dev/docs.",
+		// BUG-AUTH-006: do NOT bake a numeric cap into this sentence —
+		// the cap varies per endpoint (resource names 1-64; PAT names
+		// up to 120; team names up to 200). Each handler's `message`
+		// field carries the endpoint-specific limit; agent_action just
+		// tells the agent to read that and shorten.
+		AgentAction: "Tell the user the 'name' field is too long. Read the exact limit from `message`, shorten the value to fit, and retry — see https://instanode.dev/docs.",
 	},
 	"body_too_long": {
 		AgentAction: "Tell the user the request body exceeded the per-endpoint cap. Shrink the payload — see https://instanode.dev/docs for per-endpoint limits.",
 
@@ -393,9 +393,18 @@ func logMagicLinkSendResult(sendErr error, requestID string) {
 func (h *MagicLinkHandler) Callback(c *fiber.Ctx) error {
 	requestID := middleware.GetRequestID(c)
 
+	// BUG-API-011: accept `?token=` as a fallback for `?t=` so a user who
+	// hand-typed (or an MCP tool that guessed) the longer param name still
+	// lands on the validation branch instead of "missing its token." The
+	// canonical magic-link URL we emit always uses `?t=` (15-char-long
+	// plaintext kept short for SMS-style copy-paste); `token` is purely a
+	// recovery alias and is never advertised.
 	plaintext := strings.TrimSpace(c.Query("t"))
 	if plaintext == "" {
-		return renderAuthError(c, fiber.StatusBadRequest, "Sign-in link is missing its token", "Open the link from your email exactly as we sent it.")
+		plaintext = strings.TrimSpace(c.Query("token"))
+	}
+	if plaintext == "" {
+		return renderAuthError(c, fiber.StatusBadRequest, "Sign-in link is missing its token", "Open the link from your email exactly as we sent it — the URL should include `?t=...`.")
 	}
 
 	hash := models.HashMagicLink(plaintext)
 
@@ -0,0 +1,29 @@
+package handlers
+
+// magic_link_token_alias_test.go — BUG-API-011 regression.
+//
+// Pre-fix: GET /auth/email/callback?token=<plaintext> rendered
+// "Sign-in link is missing its token" — but the token IS present, just
+// under the longer param name. The canonical magic-link URL uses
+// `?t=<plaintext>` (kept short for SMS-style copy-paste); `?token=` is a
+// fallback alias that lets a hand-typing user / MCP tool that guessed
+// the longer name still hit the validation branch.
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMagicLink_Callback_AcceptsTokenAlias(t *testing.T) {
+	src, err := os.ReadFile("magic_link.go")
+	require.NoError(t, err, "magic_link.go must be readable from package dir")
+	body := string(src)
+
+	assert.Contains(t, body, `c.Query("t")`,
+		"BUG-API-011: canonical param `?t=` must still be read")
+	assert.Contains(t, body, `c.Query("token")`,
+		"BUG-API-011: `?token=` fallback must be read so the wrong-param-name UX no longer says 'missing'")
+}
@@ -96,6 +96,46 @@ const unauthorizedMessage = "Authentication required: missing, malformed, or exp
 // claims, invalid PAT). Kept as a single helper so adding RFC 6750
 // WWW-Authenticate headers in a future PR happens in one place.
 func respondUnauthorized(c *fiber.Ctx) error {
+	return respondUnauthorizedWithReason(c, AuthErrorMissingCredentials)
+}
+
+// AuthErrorReason names the specific 401 sub-classification an agent or
+// dashboard can branch on. The top-level `error` keyword stays `unauthorized`
+// for back-compat (any client matching on `error=="unauthorized"` continues
+// to work); the sub-code lands in a new `error_code` field. Without this an
+// agent inspecting a 401 from `/auth/me`, `/api/v1/whoami`, or any protected
+// route had no way to distinguish "I never sent credentials" from "my JWT
+// expired 30s ago" from "the signature is wrong" — they all rendered the
+// same envelope (BUG-API-035/051).
+type AuthErrorReason string
+
+const (
+	// AuthErrorMissingCredentials — no `Authorization: Bearer ...` header.
+	// Remediation: log in to mint a session token.
+	AuthErrorMissingCredentials AuthErrorReason = "missing_credentials"
+	// AuthErrorMalformedToken — header present but the value is not a
+	// well-formed JWT/PAT (parse failure, wrong shape, signature mismatch).
+	// Remediation: log in to mint a fresh token; the existing one is
+	// corrupted, not just stale.
+	AuthErrorMalformedToken AuthErrorReason = "malformed_token"
+	// AuthErrorExpiredToken — JWT parsed, signature valid, but `exp` is in
+	// the past. Remediation: log in to mint a fresh token.
+	AuthErrorExpiredToken AuthErrorReason = "expired_token"
+	// AuthErrorInvalidClaims — JWT parsed and signature valid, but a
+	// required claim (uid/tid) is empty. Remediation: log in to mint a
+	// fresh, fully-populated token.
+	AuthErrorInvalidClaims AuthErrorReason = "invalid_claims"
+	// AuthErrorRevokedSession — token's jti is in the session-revocation
+	// set (the user explicitly logged out). Remediation: log in to mint a
+	// new session.
+	AuthErrorRevokedSession AuthErrorReason = "revoked_session"
+)
+
+// respondUnauthorizedWithReason writes the canonical 401 envelope with an
+// additional `error_code` sub-field carrying the AuthErrorReason. The
+// top-level `error` keyword stays `unauthorized` so existing clients matching
+// on it keep working unchanged.
+func respondUnauthorizedWithReason(c *fiber.Ctx, reason AuthErrorReason) error {
 	// B10 P2-4 (BugBash 2026-05-20): RFC 6750 §3 requires `WWW-Authenticate:
 	// Bearer realm=...` on every 401 from a Bearer-protected resource.
 	// Pre-fix only the audience-mismatch path set the header — every other
@@ -108,6 +148,7 @@ func respondUnauthorized(c *fiber.Ctx) error {
 	return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{
 		"ok":                  false,
 		"error":               "unauthorized",
+		"error_code":          string(reason),
 		"message":             unauthorizedMessage,
 		"request_id":          GetRequestID(c),
 		"retry_after_seconds": nil,
@@ -266,7 +307,7 @@ func RequireAuth(cfg *config.Config) fiber.Handler {
 		// would have to reason about. See PR #176 refactor note.
 		header := c.Get("Authorization")
 		if len(header) < 8 || header[:7] != "Bearer " {
-			return respondUnauthorized(c)
+			return respondUnauthorizedWithReason(c, AuthErrorMissingCredentials)
 		}
 		tokenStr := header[7:]
 
@@ -276,7 +317,7 @@ func RequireAuth(cfg *config.Config) fiber.Handler {
 		if IsAPIKey(tokenStr) {
 			ok, err := AuthenticateAPIKey(c, tokenStr)
 			if err != nil || !ok {
-				return respondUnauthorized(c)
+				return respondUnauthorizedWithReason(c, AuthErrorMalformedToken)
 			}
 			return c.Next()
 		}
@@ -296,11 +337,21 @@ func RequireAuth(cfg *config.Config) fiber.Handler {
 			return []byte(cfg.JWTSecret), nil
 		}, jwt.WithValidMethods([]string{"HS256"}))
 		if err != nil || !parsed.Valid {
-			return respondUnauthorized(c)
+			// BUG-API-051: differentiate expired-vs-malformed so an agent
+			// can branch "refresh the session" from "ask the user to log
+			// in again." jwt/v4 returns *jwt.ValidationError whose Is()
+			// implementation matches the public ErrTokenExpired /
+			// ErrTokenNotValidYet sentinels; everything else is "the
+			// signature, alg, or shape was wrong" — that's malformed.
+			reason := AuthErrorMalformedToken
+			if errors.Is(err, jwt.ErrTokenExpired) || errors.Is(err, jwt.ErrTokenNotValidYet) {
+				reason = AuthErrorExpiredToken
+			}
+			return respondUnauthorizedWithReason(c, reason)
 		}
 
 		if claims.UserID == "" || claims.TeamID == "" {
-			return respondUnauthorized(c)
+			return respondUnauthorizedWithReason(c, AuthErrorInvalidClaims)
 		}
 
 		// RFC 8707 audience check — only enforced when the token actually
@@ -323,7 +374,7 @@ func RequireAuth(cfg *config.Config) fiber.Handler {
 				// Logged inside IsJTIRevoked; no additional log here to avoid duplication.
 				_ = err
 			} else if revoked {
-				return respondUnauthorized(c)
+				return respondUnauthorizedWithReason(c, AuthErrorRevokedSession)
 			}
 		}
 
@@ -443,7 +494,7 @@ func optionalAuthImpl(cfg *config.Config, strict bool) fiber.Handler {
 		}
 		if len(header) < 8 || header[:7] != "Bearer " {
 			if strict {
-				return respondUnauthorized(c)
+				return respondUnauthorizedWithReason(c, AuthErrorMalformedToken)
 			}
 			return c.Next()
 		}
@@ -470,7 +521,13 @@ func optionalAuthImpl(cfg *config.Config, strict bool) fiber.Handler {
 				// Header present but JWT is bad — reject so the caller
 				// learns their token is the problem instead of silently
 				// downgrading to anonymous. T19 P1-7.
-				return respondUnauthorized(c)
+				reason := AuthErrorMalformedToken
+				if errors.Is(err, jwt.ErrTokenExpired) || errors.Is(err, jwt.ErrTokenNotValidYet) {
+					reason = AuthErrorExpiredToken
+				} else if err == nil && parsed != nil && parsed.Valid {
+					reason = AuthErrorInvalidClaims
+				}
+				return respondUnauthorizedWithReason(c, reason)
 			}
 			return c.Next()
 		}