Deferred from PR LeanerCloud/cloud-commitments-cli#1737, where CodeRabbit raised it. Filing so the suggestion is tracked rather than lost.
What
AuthServiceInterface (internal/api/types.go) passes request bodies as any across the group and user API boundary:
CreateUserAPI(ctx context.Context, req any) (any, error)
UpdateUserAPI(ctx context.Context, actorUserID, userID string, req any) (any, error)
CreateGroupAPI(ctx context.Context, actorUserID string, req any) (any, error)
UpdateGroupAPI(ctx context.Context, actorUserID, groupID string, req any) (any, error)
Each implementation opens with a type assertion and returns fmt.Errorf("invalid request type") on mismatch, so a caller passing the wrong concrete type fails at runtime rather than at compile time.
The any signature is pre-existing design; LeanerCloud/cloud-commitments-cli#1737 only added a parameter to it. Converting to concrete types means touching every implementation and every call site of AuthServiceInterface — the real service, the authServiceAdapter in internal/server/app.go, MockAuthService, the mockAuthForExchange stub, and the handler call sites — inside a PR whose purpose was closing a p1 privilege-escalation issue (LeanerCloud/cloud-commitments-cli#1550). Mixing an interface refactor into a security review makes both harder to review, and the security fix is the time-critical half.
The original reason for any is recorded in internal/auth/service_api.go: "They use any to avoid import cycles with the api package." Any fix has to address that constraint — most likely by moving the request types into a shared package, or by defining the API-side types and converting at the adapter rather than inside the service.
Scope
internal/api/types.go — the interfaceinternal/auth/service_api.go — the four implementations plus their type assertionsinternal/server/app.go — authServiceAdapterinternal/api/mocks_test.go, internal/api/handler_ri_exchange_test.go — test doubles- handler call sites in
internal/api/handler_groups.go, handler_users.go
Worth doing as a standalone refactor with no behaviour change, so the diff is reviewable as exactly that.
Deferred from PR LeanerCloud/cloud-commitments-cli#1737, where CodeRabbit raised it. Filing so the suggestion is tracked rather than lost.
What
AuthServiceInterface(internal/api/types.go) passes request bodies asanyacross the group and user API boundary:Each implementation opens with a type assertion and returns
fmt.Errorf("invalid request type")on mismatch, so a caller passing the wrong concrete type fails at runtime rather than at compile time.Why it was not done in LeanerCloud/cloud-commitments-cli#1737
The
anysignature is pre-existing design; LeanerCloud/cloud-commitments-cli#1737 only added a parameter to it. Converting to concrete types means touching every implementation and every call site ofAuthServiceInterface— the real service, theauthServiceAdapterininternal/server/app.go,MockAuthService, themockAuthForExchangestub, and the handler call sites — inside a PR whose purpose was closing a p1 privilege-escalation issue (LeanerCloud/cloud-commitments-cli#1550). Mixing an interface refactor into a security review makes both harder to review, and the security fix is the time-critical half.The original reason for
anyis recorded ininternal/auth/service_api.go: "They use any to avoid import cycles with the api package." Any fix has to address that constraint — most likely by moving the request types into a shared package, or by defining the API-side types and converting at the adapter rather than inside the service.Scope
internal/api/types.go— the interfaceinternal/auth/service_api.go— the four implementations plus their type assertionsinternal/server/app.go—authServiceAdapterinternal/api/mocks_test.go,internal/api/handler_ri_exchange_test.go— test doublesinternal/api/handler_groups.go,handler_users.goWorth doing as a standalone refactor with no behaviour change, so the diff is reviewable as exactly that.