* 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>
* feat(cli,domain): support non-global namespace skill download
Parse namespace from skill name using -- separator (e.g.,
astroclaw--api-gateway) so users don't need --namespace flag.
Allow anonymous download for any PUBLIC skill regardless of namespace.
CLI changes:
- Add cli/src/shared/skill-name-parser.ts utility
- Update install and remove commands to parse skill name argument
- 10 unit tests covering edge cases
Domain changes:
- SkillDownloadService.isAnonymousDownloadAllowed: drop namespace
type check, only require PUBLIC visibility
- Update test to expect success for team-namespace public skill
Synced from SAAS commit 26c67e31b1221249cf9b73321d1b726d8ba6e6df
* fix(cli): use bun:test instead of vitest in skill-name-parser test
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
When --agent is provided without --scope, scope was inferred via
root.startsWith(cwd), which mislabels user roots as project when
cwd === home. Use profile.userRoots(home) membership instead, so the
candidate scope reflects the profile's intent rather than path prefix
overlap. The chosen root path itself is unchanged.
- Distinguish user vs project install scope via explicit --scope flag
- Interactive mode prompts for scope when --scope/--agent/--dir not provided
- Non-interactive bare install preserves existing behavior (backward compatible)
- Mutual exclusion: --dir cannot be combined with --scope or --agent
- Symmetric fallback: --scope user falls back to ~/.agents/skills,
--scope project falls back to <cwd>/.agents/skills
- Strict TTY check requires both stdin and stdout TTY plus no --json
- Scope-aware candidate generation avoids root.startsWith(cwd) misjudgement
when cwd === home or paths overlap
- Correct gemini-cli (.gemini/skills) and kiro-cli (.kiro/skills) paths
in install path tables across README and guide docs
- Note CLI fallback uses .agents/skills (with s) in skill protocol doc
