fix(api): tell callers why a request was forbidden (#610)

* fix(api): tell callers why a request was forbidden

The scope filter already computes an exact reason ("Missing API token
scope: skill:delete", "API token cannot access endpoint: /x") and the
access-denied handler discarded it, returning a bare "Forbidden" for
every case: missing scope, endpoint closed to API tokens, and paths
that simply don't exist. Clients cannot tell those apart, so they
guess — the published CLI reports every 403 as "token may lack
required scope", which sent us debugging token scopes for an hour when
the real causes were a revoked token and a mistyped namespace path.

The reason now rides in the response via a new error.forbidden.detail
message (en + zh), and is logged alongside the exception type.

Signed-off-by: Gal Eyal <gal.e@popai.health>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(api): safely expose API token denial reasons

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>

---------

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
gale-popai 2026-07-28 12:42:20 +03:00 committed by GitHub
parent 1d679c526a
commit d977ea9dc4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 302 additions and 18 deletions

View file

@ -52,6 +52,11 @@ export interface DryRunResponse {
resolvedVersion: string | null
}
interface ErrorEnvelope {
msg?: unknown
requestId?: unknown
}
export class SkillHubClient {
constructor(
readonly registry: string,
@ -88,9 +93,12 @@ export class SkillHubClient {
} catch {
throw new CliError('registry unreachable', EXIT.network, { registry: this.registry, next: 'check network or pass --registry' })
}
if (response.status === 401 || response.status === 403) {
if (response.status === 401) {
throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' })
}
if (response.status === 403) {
throw await this.createAccessDeniedError(response)
}
if (response.status === 404) {
throw new CliError('skill or version not found', EXIT.generic, { registry: this.registry })
}
@ -155,7 +163,7 @@ export class SkillHubClient {
throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' })
}
if (response.status === 403) {
throw new CliError('access denied — token may lack required scope', EXIT.auth, { registry: this.registry, next: 'regenerate token with required scopes or run `skillhub login`' })
throw await this.createAccessDeniedError(response)
}
if (response.status === 404) {
throw new CliError('resource not found', EXIT.generic, { registry: this.registry })
@ -172,6 +180,26 @@ export class SkillHubClient {
return body.data as T
}
private async createAccessDeniedError(response: Response): Promise<CliError> {
const error = await this.readErrorEnvelope(response)
return new CliError(error.message ?? 'access denied', EXIT.auth, {
registry: this.registry,
...(error.requestId ? { requestId: error.requestId } : {})
})
}
private async readErrorEnvelope(response: Response): Promise<{ message?: string; requestId?: string }> {
try {
const body = await response.json() as ErrorEnvelope
return {
...(typeof body.msg === 'string' && body.msg.trim() ? { message: body.msg } : {}),
...(typeof body.requestId === 'string' && body.requestId.trim() ? { requestId: body.requestId } : {})
}
} catch {
return {}
}
}
private headers(): HeadersInit {
return this.token ? { Authorization: `Bearer ${this.token}` } : {}
}

View file

@ -30,6 +30,9 @@ export function renderError(error: unknown, json: boolean): string {
if (typeof cliError.details.path === 'string') {
lines.push(`Context: path ${cliError.details.path}`)
}
if (typeof cliError.details.requestId === 'string') {
lines.push(`Request ID: ${cliError.details.requestId}`)
}
if (typeof cliError.details.next === 'string') {
lines.push(`Next: ${cliError.details.next}`)
}

View file

@ -22,7 +22,7 @@ export function createFakeRegistry(handlers: Record<string, FakeHandler>) {
/**
* Controls how a specific endpoint behaves when a failure is injected:
* 'auth' => 401 { code: 401, message: 'unauthorized' }
* 'forbidden' => 403 { code: 403, message: 'forbidden' }
* 'forbidden' => 403 with a standard SkillHub error envelope
* 'not_found' => 404 { code: 404, message: 'not found' }
* 'server_error' => 500 { code: 500, message: 'internal error' }
* 'network' => handler throws, causing fetch() to reject with a TypeError
@ -34,7 +34,11 @@ function failureResponse(mode: FailureMode): Response {
case 'auth':
return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 })
case 'forbidden':
return Response.json({ code: 403, message: 'forbidden' }, { status: 403 })
return Response.json({
code: 403,
msg: 'API token is missing required scope: skill:publish',
requestId: 'req-test-forbidden'
}, { status: 403 })
case 'not_found':
return Response.json({ code: 404, message: 'not found' }, { status: 404 })
case 'server_error':

View file

@ -172,5 +172,6 @@ describe('publish --dry-run', () => {
expect(result.exitCode).toBe(2)
expect(result.stderr).toContain('scope')
expect(result.stderr).toContain('Request ID: req-test-forbidden')
})
})

View file

@ -37,12 +37,21 @@ describe('SkillHubClient', () => {
})
test('download() throws auth error on 403', async () => {
const fetchImpl = (async () => new Response(null, { status: 403 })) as unknown as typeof fetch
const fetchImpl = (async () => Response.json({
code: 403,
msg: 'API token is missing required scope: skill:read',
requestId: 'req-download'
}, { status: 403 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
const err = expect(client.download('ns', 'slug')).rejects
await err.toBeInstanceOf(CliError)
await err.toHaveProperty('message', 'authentication failed')
await err.toHaveProperty('exitCode', EXIT.auth)
await expect(client.download('ns', 'slug')).rejects.toMatchObject({
message: 'API token is missing required scope: skill:read',
exitCode: EXIT.auth,
details: {
registry: 'http://registry.test',
requestId: 'req-download'
}
})
})
test('download() throws not-found error on 404', async () => {
@ -159,6 +168,35 @@ describe('SkillHubClient', () => {
// --- handleJsonResponse() non-2xx classification ---
test('whoami() surfaces server reason and request ID on 403', async () => {
const fetchImpl = (async () => Response.json({
code: 403,
msg: 'API token cannot access endpoint: /api/cli/v1/whoami',
requestId: 'req-610'
}, { status: 403 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
await expect(client.whoami()).rejects.toMatchObject({
message: 'API token cannot access endpoint: /api/cli/v1/whoami',
exitCode: EXIT.auth,
details: {
registry: 'http://registry.test',
requestId: 'req-610'
}
})
})
test('whoami() falls back to generic access denied when 403 body is invalid', async () => {
const fetchImpl = (async () => new Response('not-json', { status: 403 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
await expect(client.whoami()).rejects.toMatchObject({
message: 'access denied',
exitCode: EXIT.auth,
details: { registry: 'http://registry.test' }
})
})
test('whoami() throws generic error on 500', async () => {
const fetchImpl = (async () => new Response(null, { status: 500 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)

View file

@ -16,11 +16,13 @@ describe('renderError', () => {
test('renders human error without stack trace', () => {
const error = new CliError('registry unreachable', 3, {
registry: 'https://registry.example.com',
requestId: 'req-610',
next: 'check network or pass --registry'
})
expect(renderError(error, false)).toBe([
'Error: registry unreachable',
'Context: registry https://registry.example.com',
'Request ID: req-610',
'Next: check network or pass --registry'
].join('\n'))
})

View file

@ -379,6 +379,7 @@ API Token 仍保留但定位从“CLI 唯一认证方式”调整为“平台
- 校验:从 `Authorization: Bearer <token>` 提取 → 哈希比对 → 加载关联用户 → 检查用户状态
- 失败闭合:公共读接口只有在缺少 `Authorization` 头时才按匿名访问处理;只要出现 Bearer 凭证,空值、格式错误、未知、过期、已吊销、用户缺失或用户禁用均返回 401不能回退为匿名访问
- 作用域:`skill:read`, `skill:publish`, `skill:delete`, `token:manage`
- 拒绝原因API Token 缺少作用域或不能访问某个接口时403 响应返回本地化的安全原因和 `requestId`;其他授权失败仍返回通用信息,避免暴露内部异常
> **一期作用域说明(非最小权限)**:一期 Token 作用域为粗粒度动作级别,不与 namespace 绑定。Token 继承用户的全部权限——如果用户是某个 namespace 的 MEMBER则该用户的任何 Token只要包含 `skill:publish` scope都可以向该 namespace 发布技能。这是有意的一期简化,不满足最小权限原则。后续版本计划引入 namespace 级别的 Token 作用域限定(如 `namespace:ai-team:skill:publish`),或通过 `api_token_scope` 子表实现 Token 与 namespace 的绑定。

View file

@ -83,6 +83,8 @@ skillhub login --token sk_xxx --registry https://skillhub.example.com
`login` validates the token, stores it in `~/.skillhub/credentials.json`, and writes the registry to `~/.skillhub/config.json`.
When an API-token request is denied, the CLI shows the safe reason returned by the server and its `Request ID`. Use that ID to correlate the failure with server logs. Other authorization failures continue to use a generic message.
### Check Current Identity
```bash

View file

@ -83,6 +83,8 @@ skillhub login --token sk_xxx --registry https://skillhub.example.com
`login` 会验证 token 有效性,然后将 token 存储到 `~/.skillhub/credentials.json`,同时将 registry 写入 `~/.skillhub/config.json`
API Token 请求被拒绝时CLI 会显示服务端返回的具体原因和 `Request ID`。排查问题时可使用该 ID 对照服务端日志;非 API Token 的授权失败仍只显示通用信息。
### 查看当前身份
```bash

View file

@ -1,6 +1,7 @@
package com.iflytek.skillhub.security;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.iflytek.skillhub.auth.token.ApiTokenAccessDeniedException;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import jakarta.servlet.http.HttpServletRequest;
@ -38,14 +39,25 @@ public class ApiAccessDeniedHandler implements AccessDeniedHandler {
public void handle(HttpServletRequest request,
HttpServletResponse response,
AccessDeniedException accessDeniedException) throws IOException {
ApiTokenAccessDeniedException apiTokenException =
accessDeniedException instanceof ApiTokenAccessDeniedException typedException
? typedException
: null;
logger.info(
"Forbidden API request [requestId={}, method={}, path={}, reason={}]",
"Forbidden API request [requestId={}, method={}, path={}, reason={}, detail={}]",
MDC.get("requestId"),
request.getMethod(),
sensitiveLogSanitizer.sanitizeRequestTarget(request),
accessDeniedException.getClass().getSimpleName()
accessDeniedException.getClass().getSimpleName(),
apiTokenException != null ? apiTokenException.getMessage() : null
);
ApiResponse<Void> body = apiResponseFactory.error(403, "error.forbidden");
ApiResponse<Void> body = apiTokenException != null
? apiResponseFactory.error(
403,
apiTokenException.getMessageCode(),
apiTokenException.getMessageArgs()
)
: apiResponseFactory.error(403, "error.forbidden");
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
objectMapper.writeValue(response.getOutputStream(), body);

View file

@ -48,6 +48,8 @@ error.auth.sessionBootstrap.providerUnsupported=Unsupported session bootstrap pr
error.auth.sessionBootstrap.notAuthenticated=No authenticated external session found
error.badRequest=Invalid request
error.forbidden=Forbidden
error.apiToken.scope.missing=API token is missing required scope: {0}
error.apiToken.endpoint.unsupported=API token cannot access endpoint: {0}
error.request.timeout=Request timed out
error.rateLimit.exceeded=Rate limit exceeded
error.storage.unavailable=Object storage is temporarily unavailable. Please try again later.

View file

@ -48,6 +48,8 @@ error.auth.sessionBootstrap.providerUnsupported=不支持的会话引导提供
error.auth.sessionBootstrap.notAuthenticated=未检测到已认证的外部会话
error.badRequest=请求参数不合法
error.forbidden=没有权限执行该操作
error.apiToken.scope.missing=API 令牌缺少所需权限范围:{0}
error.apiToken.endpoint.unsupported=API 令牌无法访问接口:{0}
error.request.timeout=请求超时
error.rateLimit.exceeded=请求过于频繁,请稍后再试
error.storage.unavailable=对象存储暂时不可用,请稍后再试

View file

@ -0,0 +1,137 @@
package com.iflytek.skillhub.security;
import static org.assertj.core.api.Assertions.assertThat;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.iflytek.skillhub.auth.token.ApiTokenScopeFilter;
import com.iflytek.skillhub.auth.token.ApiTokenScopeService;
import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry;
import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import jakarta.servlet.FilterChain;
import java.time.Clock;
import java.time.Instant;
import java.time.ZoneOffset;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.slf4j.MDC;
import org.springframework.context.i18n.LocaleContextHolder;
import org.springframework.context.support.ResourceBundleMessageSource;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContextHolder;
class ApiAccessDeniedHandlerTest {
private final ObjectMapper objectMapper = new ObjectMapper().findAndRegisterModules();
private ApiAccessDeniedHandler handler;
@BeforeEach
void setUp() {
ResourceBundleMessageSource messageSource = new ResourceBundleMessageSource();
messageSource.setBasename("messages");
messageSource.setDefaultEncoding("UTF-8");
ApiResponseFactory responseFactory = new ApiResponseFactory(
messageSource,
Clock.fixed(Instant.parse("2026-07-28T00:00:00Z"), ZoneOffset.UTC)
);
handler = new ApiAccessDeniedHandler(
objectMapper,
responseFactory,
new SensitiveLogSanitizer()
);
MDC.put("requestId", "req-610");
LocaleContextHolder.setLocale(Locale.ENGLISH);
}
@AfterEach
void tearDown() {
MDC.clear();
LocaleContextHolder.resetLocaleContext();
SecurityContextHolder.clearContext();
}
@Test
void shouldExposeLocalizedApiTokenScopeReasonAndRequestId() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/publish");
MockHttpServletResponse response = new MockHttpServletResponse();
ApiTokenScopeService scopeService =
new ApiTokenScopeService(objectMapper, new RouteSecurityPolicyRegistry());
ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler);
PlatformPrincipal principal = new PlatformPrincipal(
"user-1",
"Alice",
"alice@example.com",
"",
"api_token",
Set.of("USER")
);
SecurityContextHolder.getContext().setAuthentication(
new UsernamePasswordAuthenticationToken(
principal,
null,
List.of(new SimpleGrantedAuthority("SCOPE_skill:read"))
)
);
FilterChain chain = (servletRequest, servletResponse) -> {
throw new AssertionError("Denied request must not continue");
};
filter.doFilter(request, response, chain);
JsonNode body = objectMapper.readTree(response.getContentAsByteArray());
assertThat(response.getStatus()).isEqualTo(403);
assertThat(body.path("msg").asText())
.isEqualTo("API token is missing required scope: skill:publish");
assertThat(body.path("requestId").asText()).isEqualTo("req-610");
}
@Test
void shouldTranslateSafeApiTokenReason() throws Exception {
LocaleContextHolder.setLocale(Locale.SIMPLIFIED_CHINESE);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/cli/v1/whoami");
MockHttpServletResponse response = new MockHttpServletResponse();
ApiTokenScopeService scopeService =
new ApiTokenScopeService(objectMapper, new RouteSecurityPolicyRegistry());
ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler);
PlatformPrincipal principal = new PlatformPrincipal(
"user-1",
"Alice",
"alice@example.com",
"",
"api_token",
Set.of("USER")
);
SecurityContextHolder.getContext().setAuthentication(
new UsernamePasswordAuthenticationToken(principal, null, List.of())
);
filter.doFilter(request, response, (servletRequest, servletResponse) -> {
throw new AssertionError("Denied request must not continue");
});
JsonNode body = objectMapper.readTree(response.getContentAsByteArray());
assertThat(body.path("msg").asText())
.isEqualTo("API 令牌无法访问接口:/api/cli/v1/whoami");
}
@Test
void shouldHideGenericAccessDeniedExceptionMessage() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/v1/admin");
MockHttpServletResponse response = new MockHttpServletResponse();
handler.handle(request, response, new AccessDeniedException("internal authorization detail"));
JsonNode body = objectMapper.readTree(response.getContentAsByteArray());
assertThat(body.path("msg").asText()).isEqualTo("Forbidden");
assertThat(response.getContentAsString()).doesNotContain("internal authorization detail");
}
}

View file

@ -0,0 +1,42 @@
package com.iflytek.skillhub.auth.token;
import org.springframework.security.access.AccessDeniedException;
/**
* Marks an API-token authorization failure whose structured reason is safe to expose to clients.
*/
public final class ApiTokenAccessDeniedException extends AccessDeniedException {
private final String messageCode;
private final Object[] messageArgs;
private ApiTokenAccessDeniedException(String logMessage, String messageCode, Object... messageArgs) {
super(logMessage);
this.messageCode = messageCode;
this.messageArgs = messageArgs.clone();
}
static ApiTokenAccessDeniedException missingScope(String requiredScope) {
return new ApiTokenAccessDeniedException(
"Missing API token scope: " + requiredScope,
"error.apiToken.scope.missing",
requiredScope
);
}
static ApiTokenAccessDeniedException unsupportedEndpoint(String path) {
return new ApiTokenAccessDeniedException(
"API token cannot access endpoint: " + path,
"error.apiToken.endpoint.unsupported",
path
);
}
public String getMessageCode() {
return messageCode;
}
public Object[] getMessageArgs() {
return messageArgs.clone();
}
}

View file

@ -5,7 +5,6 @@ import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.context.SecurityContextHolder;
@ -59,11 +58,10 @@ public class ApiTokenScopeFilter extends OncePerRequestFilter {
return;
}
accessDeniedHandler.handle(
request,
response,
new AccessDeniedException(decision.message())
);
ApiTokenAccessDeniedException exception = decision.requiredScope() != null
? ApiTokenAccessDeniedException.missingScope(decision.requiredScope())
: ApiTokenAccessDeniedException.unsupportedEndpoint(request.getRequestURI());
accessDeniedHandler.handle(request, response, exception);
}
@Override

View file

@ -17,8 +17,10 @@ import org.springframework.security.web.access.AccessDeniedHandler;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
@ -38,7 +40,9 @@ class ApiTokenScopeFilterTest {
@Test
void shouldDenyApiTokenWithoutRequiredScope() throws Exception {
AtomicReference<Exception> deniedException = new AtomicReference<>();
AccessDeniedHandler handler = (request, response, accessDeniedException) -> {
deniedException.set(accessDeniedException);
response.sendError(HttpServletResponse.SC_FORBIDDEN, accessDeniedException.getMessage());
};
ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler);
@ -69,6 +73,12 @@ class ApiTokenScopeFilterTest {
assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus());
assertTrue(response.getErrorMessage().contains("Missing API token scope: skill:publish"));
ApiTokenAccessDeniedException exception = assertInstanceOf(
ApiTokenAccessDeniedException.class,
deniedException.get()
);
assertEquals("error.apiToken.scope.missing", exception.getMessageCode());
assertEquals("skill:publish", exception.getMessageArgs()[0]);
verify(chain, never()).doFilter(request, response);
}