* 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>
Covers the new "access denied — token may lack required scope" error
path with a fake-registry 'forbidden' failure mode. Prevents the
improved 403 message from regressing silently.
- Fix test fixture: warnings-only response now uses valid=false to
match real backend behavior (warnings make dry-run invalid)
- Distinguish 403 from 401 in CLI error messages: 403 now says
"access denied — token may lack required scope" with a hint to
regenerate the token, rather than the generic "authentication failed"
Fix three blockers and one contract drift issue surfaced in code review:
1. API token policy: add skill:publish scope policy and authentication
policy for /api/cli/v1/skills/*/publish/validate. Without these the
AntPathMatcher pattern /publish would not cover /publish/validate,
so Bearer-token requests would be rejected by the scope filter.
2. Warnings semantics: dry-run now treats warnings as making valid=false.
The CLI publish flow uses confirmWarnings=false, so the real publish
rejects any warnings; dry-run must mirror that to avoid false positives.
3. Visibility parameter: validate endpoint now accepts the same
visibility multipart field as publish. The CLI forwards --visibility
so invalid values are caught at dry-run time rather than at publish.
4. Schema drift: resolvedSlug and resolvedVersion are nullable in
practice (returned as null when validation fails before resolution).
Updated schema.d.ts to reflect string | null instead of optional string.
Tests added:
- RouteSecurityPolicyRegistryTest: validate endpoint scope check
- CliDryRunValidateTest: custom + invalid visibility cases
- publish-dry-run.test.ts: --visibility forwarded to server
- Exit non-zero (code 6) when --dry-run validation fails, enabling
CI/CD pipeline integration
- Add archived skill check: dry-run now detects when the publisher's
own skill is archived
- Add version-exists check: dry-run now detects when the resolved
version is already published
- Use StandardCharsets.UTF_8 for SKILL.md content parsing
Add a validate-only endpoint (POST /api/cli/v1/skills/{namespace}/publish/validate)
that runs the full pre-publish validation chain without persisting anything.
This allows developers to check their package locally before actual publishing.
The validation covers:
- SKILL.md existence and frontmatter parsing (name, description required)
- File extension whitelist and size limits
- Credential leak scanning with line-number precision
- Slug generation and name conflict detection
CLI usage: `skillhub publish <path> --dry-run`
Closes#429
