Skip to content

Commit 5577ccb

Browse files
caster-Qclaude
andcommitted
chore(auth): v3.4 batch — 4 nit cleanup + drop JWT-era 410 stubs
Picks up 4 of the v3.3.6 §7 follow-up nit list + removes 2 leftover JWT-era compatibility stubs. All in octo-server, no cross-repo coupling. Strict scope per caster's principle: no compat for nonexistent clients post-Phase 4. ## Changes (5 items, all runtime-branch-exclusive surfaces) ### nit-3 — botToken empty Bearer guard (bot_provision/bot_api.go) Empty Bearer guard after TrimPrefix. Behavior alignment (early reject) with verify-api-key's empty-field guard — robustness/symmetry, not client compatibility. ### nit-6 — Remove unreachable botUID == "" dead code (bot_provision/bot_api.go) gin's :uid param cannot be empty after the route matches; the guard was carried over from the legacy JWT handler. ### nit-10 — Delete redundant deps_test.go (user/) zz_external_setup_test.go's internal blank-import is a strict superset of deps_test.go's 4-module subset. Updated zz header with supersedes note. runtime-branch-only — main keeps deps_test.go (no zz on main). ### nit-11 — Fix stale test header (user/api_verify_apikey_test.go) "6 cases" → 15 cases enumeration. ### Drop JWT-era 410 Gone stubs (bot_provision/bot_api.go) Removed `r.GET("/.well-known/jwks.json", gone410Handler(...))` + `r.POST("/v1/auth/token", gone410Handler(...))` + the `gone410Handler` helper + 2 `TestGone410_*` test cases. These stubs were registered to give pre-Phase 4 JWT-era daemon clients a 410 + migration body instead of gin's default 404. But JWT was removed in Phase 4 (决策一+二) and no real client ever ran the pre-removal version (PoC-only). Compat scaffolding for nonexistent clients — dropping it per caster's principle. gin default 404 on unregistered URLs is functionally equivalent for the actually-zero caller population. Same reasoning as why nit-4 (JWT-shaped Bearer in-handler hint) was also dropped — both branches assumed a JWT-era caller that doesn't exist. ## Impact on main branch — 0 All 5 changes touch runtime-branch-exclusive surfaces: - bot_provision/ module: does not exist on main - /v1/auth/verify-api-key test: runtime-branch-only - zz_external_setup_test.go: runtime-branch-only; deleting deps_test.go on runtime branch leaves main's deps_test.go untouched - 410 stubs were registered by bot_provision which doesn't exist on main ## Verification - `go build ./...` PASS - `go vet ./modules/user/... ./modules/bot_provision/...` PASS - `go test -count=1 -p 1 -run 'TestAuthVerify|TestBotToken|TestGone410' ./modules/user/... ./modules/bot_provision/...` PASS (user 3.8s, bot_provision 2.0s — TestGone410_* skipped since deleted) - `go test -race -count=1 -p 1 -run 'TestAuthVerify|TestBotToken' ./modules/user/... ./modules/bot_provision/...` PASS (user 7.0s, bot_provision 3.6s) - 4 TestDestroyApply_* failures observed during full-package runs are pre-existing flaky (verified on clean origin/feat/agent-runtime without this PR) — redis login-locked state leaks across tests. ## State-space scan (D1-D13) - D1 primitive call-site coverage: not applicable — no SQL changes - D4 response shape: no changes - D5 cross-PR contract: no changes - D11 revocation chain: explicitly NOT extended. Bot retention after owner ban remains per main branch semantic. ## Out of scope - nit-5 owned_bots shape unify (cross-repo D10) - nit-7 constant-time api_key compare (SQL semantic change) - nit-8 account-ban gate on owned-bot loaders (reverted; preserves main semantic) - nit-9 robot composite index (separate migration PR) - verifyCache sha256-ize redis key (security polish) - nit-4 JWT-shaped Bearer in-handler hint (reverted; same reasoning as 410 stub removal — no real JWT-era client) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f39d91f commit 5577ccb

5 files changed

Lines changed: 18 additions & 117 deletions

File tree

modules/bot_provision/bot_api.go

Lines changed: 6 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"github.com/Mininglamp-OSS/octo-lib/pkg/wkhttp"
2828
"github.com/Mininglamp-OSS/octo-server/modules/botfather"
2929
octoredis "github.com/Mininglamp-OSS/octo-server/pkg/redis"
30-
"github.com/gin-gonic/gin"
3130
rd "github.com/go-redis/redis"
3231
"go.uber.org/zap"
3332
)
@@ -115,16 +114,16 @@ func (a *BotProvision) botToken(c *wkhttp.Context) {
115114
return
116115
}
117116
apiKey := strings.TrimPrefix(auth, "Bearer ")
117+
if apiKey == "" {
118+
c.ResponseErrorWithStatus(errors.New("empty Bearer token"), http.StatusUnauthorized)
119+
return
120+
}
118121
callerUID, callerSpace, err := a.resolveAPIKey(apiKey)
119122
if err != nil {
120123
c.ResponseErrorWithStatus(errors.New("invalid api_key"), http.StatusUnauthorized)
121124
return
122125
}
123126
botUID := c.Param("uid")
124-
if botUID == "" {
125-
c.ResponseError(errors.New("uid required"))
126-
return
127-
}
128127
type row struct {
129128
BotToken string `db:"bot_token"`
130129
CreatorUID string `db:"creator_uid"`
@@ -187,17 +186,11 @@ var readRand = defaultReadRand
187186
var hexEncode = defaultHexEncode
188187

189188
// Route mounts the two bot endpoints. JWT exchange + JWKS endpoints have
190-
// been removed (合并 plan 决策一+二 Phase 4) — daemon/web now hit
191-
// fleet/matter directly with api_key/session tokens.
189+
// been removed in Phase 4 — daemon/web now hit fleet/matter directly with
190+
// api_key/session tokens.
192191
//
193192
// POST /v1/bot/mint — web session auth (octo-lib session middleware)
194193
// GET /v1/bot/:uid/token — daemon api_key Bearer (validated inline)
195-
//
196-
// Also registers 410 Gone stubs for removed legacy auth endpoints so old
197-
// callers get a deterministic deprecation signal instead of a 404 from an
198-
// unregistered path. The previous JWT/JWKS pair lived under these paths
199-
// and was dropped in 决策一+二 Phase 4; the stubs document the change for
200-
// any straggler client that hasn't been updated.
201194
func (a *BotProvision) Route(r *wkhttp.WKHttp) {
202195
authGroup := r.Group("/v1", a.ctx.AuthMiddleware(r))
203196
authGroup.POST("/bot/mint", a.mintBot)
@@ -220,44 +213,4 @@ func (a *BotProvision) Route(r *wkhttp.WKHttp) {
220213
}))
221214
verifyLimit := r.StrictIPRateLimitMiddleware(rlCtx, rlRedis, "verify", 1000.0/60, 100)
222215
r.GET("/v1/bot/:uid/token", verifyLimit, a.botToken)
223-
224-
// Removed legacy auth endpoints (决策一+二 Phase 4). 410 Gone so a
225-
// straggler client distinguishes "endpoint was removed by design" from
226-
// "wrong URL / typo". Each stub returns a stable JSON body pointing at
227-
// the replacement path so client owners can fix in place.
228-
//
229-
// v3.3.2 (Jerry-Xin R4 nit, caster local review): Sunset/Deprecation
230-
// headers dropped. v3.3.1 hardcoded both dates wrong — Sunset said
231-
// "Fri, 13 Jun 2026" but that day is actually Saturday; Deprecation
232-
// said "@1749427200" which resolves to 2025-06-09, off by a year.
233-
// Fixing the dates would just invite the next off-by-one when push
234-
// slips. There's no deploy-pipeline substitution mechanism in this
235-
// repo to keep them current, and zero current consumers depend on
236-
// either header. The 410 status + structured JSON body remain — that
237-
// is the actionable signal for both humans and automated clients.
238-
// If we ever want Sunset back, deploy-time substitution must land
239-
// first (separate PR).
240-
r.GET("/.well-known/jwks.json", gone410Handler(
241-
"JWKS endpoint removed — fleet/matter no longer verify JWTs locally. "+
242-
"Use POST /v1/auth/verify (session) or /v1/auth/verify-api-key (daemon) instead.",
243-
))
244-
r.POST("/v1/auth/token", gone410Handler(
245-
"Token exchange endpoint removed — daemon no longer exchanges api_key for a JWT. "+
246-
"Send api_key directly as Authorization: Bearer to fleet/matter; they will "+
247-
"call /v1/auth/verify-api-key for validation.",
248-
))
249-
}
250-
251-
// gone410Handler returns a wkhttp handler that always responds with HTTP 410
252-
// Gone + a structured JSON body describing why the endpoint was removed and
253-
// where to migrate. Sunset/Deprecation headers were considered (RFC 8594 /
254-
// draft-ietf-httpapi-deprecation) but dropped in v3.3.2 — see Route()
255-
// comment above for the reasoning.
256-
func gone410Handler(reason string) func(*wkhttp.Context) {
257-
return func(c *wkhttp.Context) {
258-
c.AbortWithStatusJSON(http.StatusGone, gin.H{
259-
"error": "gone",
260-
"message": reason,
261-
})
262-
}
263216
}

modules/bot_provision/bot_api_test.go

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -251,56 +251,6 @@ func TestBotToken_DisabledSpace_401(t *testing.T) {
251251
"api_key bound to a disabled space (s.status=0) must 401 even on daemon path — v3.3.3 §E")
252252
}
253253

254-
// ─────────────────────────────────────────────────────────────────────
255-
// 410 Gone stubs (v3.3.2 reduced to status + body only — Sunset/Deprecation
256-
// headers dropped, see bot_api.go Route() comment). The v3.3.1 hardcoded
257-
// dates were both wrong (Sunset said Fri but the date is Saturday;
258-
// Deprecation @1749427200 resolves to 2025-06-09 not 2026), and there is
259-
// no deploy-pipeline substitution to keep them fresh. Body still carries
260-
// the actionable migration pointer.
261-
// ─────────────────────────────────────────────────────────────────────
262-
263-
func doRaw(t *testing.T, s *server.Server, method, path string) *httptest.ResponseRecorder {
264-
t.Helper()
265-
w := httptest.NewRecorder()
266-
req, err := http.NewRequest(method, path, nil)
267-
require.NoError(t, err)
268-
s.GetRoute().ServeHTTP(w, req)
269-
return w
270-
}
271-
272-
func TestGone410_JWKS_ReturnsStatusAndBody(t *testing.T) {
273-
s, _ := testutil.NewTestServer()
274-
w := doRaw(t, s, "GET", "/.well-known/jwks.json")
275-
276-
require.Equal(t, http.StatusGone, w.Code, "JWKS endpoint must respond 410")
277-
278-
// v3.3.2: Sunset/Deprecation headers dropped — assert they are NOT
279-
// emitted. Locks the no-header decision in place so a future
280-
// "let's re-add Sunset" change has to update this test, and at that
281-
// point reviewer can confirm the constants are actually correct.
282-
assert.Empty(t, w.Header().Get("Sunset"), "Sunset header must NOT be set (v3.3.2 drop)")
283-
assert.Empty(t, w.Header().Get("Deprecation"), "Deprecation header must NOT be set (v3.3.2 drop)")
284-
285-
body := w.Body.String()
286-
assert.Contains(t, body, "JWKS endpoint removed", "body must explain why + where to migrate")
287-
assert.Contains(t, body, "verify-api-key", "body must point at the replacement endpoint")
288-
}
289-
290-
func TestGone410_TokenExchange_ReturnsStatusAndBody(t *testing.T) {
291-
s, _ := testutil.NewTestServer()
292-
w := doRaw(t, s, "POST", "/v1/auth/token")
293-
294-
require.Equal(t, http.StatusGone, w.Code, "token exchange endpoint must respond 410")
295-
296-
assert.Empty(t, w.Header().Get("Sunset"), "Sunset header must NOT be set (v3.3.2 drop)")
297-
assert.Empty(t, w.Header().Get("Deprecation"), "Deprecation header must NOT be set (v3.3.2 drop)")
298-
299-
body := w.Body.String()
300-
assert.Contains(t, body, "Token exchange endpoint removed")
301-
assert.Contains(t, body, "Authorization: Bearer")
302-
}
303-
304254
// v3.3.6 §P1 regression — yujiawei R2 P1: account ban MUST revoke
305255
// botToken (daemon api_key path). resolveAPIKey → assertSpaceMember,
306256
// which now joins `user` ON u.status=1. mintBot is NOT separately

modules/user/api_verify_apikey_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@ import (
1616
)
1717

1818
// authVerifyAPIKey: POST /v1/auth/verify-api-key
19-
// 合并 plan §3 covers 6 cases: valid / unknown / owner-left-space /
20-
// legacy-empty-space / missing-field / multi-space.
19+
// 15 test cases covering: valid / unknown / owner-left-space /
20+
// non-active-membership / legacy-empty-space / disabled-space /
21+
// missing-field / multi-space / no-include (default shape) /
22+
// with-include (owned_bots map) / owned-bots only-in-bound-space /
23+
// owned-bots empty / owned-bots filters-disabled-bot /
24+
// account-banned (v3.3.6 §P1) / include-context DB-error fail-secure.
2125

2226
const (
2327
testAPIKeySpaceA = "verify_apikey_space_a"

modules/user/deps_test.go

Lines changed: 0 additions & 12 deletions
This file was deleted.

modules/user/zz_external_setup_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818
// `user` test set; adding a second would conflict ("multiple definitions
1919
// of TestMain"). A bare blank-import in any test file is enough to
2020
// trigger init() at test binary load.
21+
//
22+
// Supersedes the deprecated deps_test.go (removed in v3.4 cleanup): that
23+
// file only blank-imported a 4-module subset (base/botfather/group/robot)
24+
// which is a strict subset of what `internal` already pulls. Keeping
25+
// both was redundant — this single file fully covers the migration
26+
// registry requirement.
2127
package user_test
2228

2329
import (

0 commit comments

Comments
 (0)