* fix(compat): prioritize PUBLIC/GLOBAL skills in legacy slug lookup
When multiple skills share the same slug across namespaces,
findByLegacySlug now prefers PUBLIC visibility and GLOBAL
namespace over NAMESPACE_ONLY/PRIVATE ones, so plain slug
lookups resolve to the most accessible skill. Namespaces are
batch-fetched via findByIdIn to avoid N+1 database queries.
Signed-off-by: wurongjie <wurongjie@uniontech.com>
* fix(compat): prefer published legacy slug candidates
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
---------
Signed-off-by: wurongjie <wurongjie@uniontech.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* feat(api): return skill labels from the skill listing endpoints on request
Skill labels were reachable only one skill at a time, through
/api/{v1,web}/skills/{namespace}/{slug}/labels, so a client rendering a list had
to issue a follow-up request per row.
Add includeLabels=true to GET /api/v1/skills and GET /api/web/skills. The labels
array is populated only when the parameter is set and left out of the payload
otherwise, so existing responses are byte-identical.
Labels for the whole page are resolved by SkillLabelProjectionService in three
queries — assignments, definitions, translations — rather than three per skill.
Closes#710
Signed-off-by: FenjuFu <fufenjupku@gmail.com>
* fix(api): use include parameter for skill labels
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* test(api): reject unsupported include before search
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
---------
Signed-off-by: FenjuFu <fufenjupku@gmail.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
RouteSecurityPolicyRegistry keeps two policy lists — AUTHORIZATION_POLICIES for
session/cookie access and API_TOKEN_POLICIES for Bearer tokens — with nothing
keeping them in step. Routes the authorization list opens but the token list
never registers fall through to the catch-all and answer
API token cannot access endpoint: <path>.
Register the routes reported in #713 (/api/v1/labels, the star and rating
writes) plus the same-class gaps for /api/v1/auth/methods and paths below
/api/v1/download, and add a guard test that walks the authorization list and
fails when a route is neither token-reachable nor declared session-only.
DELETE /api/v1/skills/{id}/star also matched the SUPER_ADMIN rule for
DELETE /api/v1/skills/*/*, so un-starring was refused for ordinary accounts on
the session path too. Star and rating writes now have their own authorization
entries ahead of that rule.
Closes#713
Signed-off-by: FenjuFu <fufenjupku@gmail.com>
* fix(security): publish scan task after transaction commit
SecurityScanService.triggerScan is @Transactional but published the Redis
Stream scan task inline, before the transaction committed. The stream
consumer could receive the task before the skill_version / security_audit
rows were visible, fail with "SkillVersion not found" / "SecurityAudit not
found", exhaust its immediate retries while the publishing transaction was
still open, and leave the committed version stuck in SCANNING.
Defer the publish to an afterCommit transaction synchronization so the
consumer only ever sees the task once the rows are committed and visible; on
rollback the task is never published. Falls back to an inline publish when
called outside a transaction.
Closes#612
Signed-off-by: FenjuFu <fufenjupku@gmail.com>
* test(security): cover scan task after-commit publishing
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* refactor(security): hide scan publish transaction callback
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
---------
Signed-off-by: FenjuFu <fufenjupku@gmail.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Downloading a skill whose slug is non-ASCII (e.g. a Chinese name) through
the ClawHub CLI compatibility route failed: the 302 Location header was
built by string-concatenating the raw slug, and Tomcat encodes header
values as ISO-8859-1, so a character outside 0-255 makes it drop the
Location header entirely and the download breaks. The skillhub CLI path
was unaffected because it doesn't go through this redirect.
Build the Location with UriComponentsBuilder.pathSegment(...).encode(), so
each segment is percent-encoded while the '/' separators stay literal.
"需求" becomes %E9%9C%80%E6%B1%82 and the header is ISO-8859-1-writable.
Fixes#658
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
Remove terminal review tasks before deleting an allowed skill version. Lock all versions of the aggregate in stable order so concurrent deletes preserve the last-version invariant and return business errors instead of 500 responses.
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
OAuth2AuthorizationRequestRedirectFilter invokes the resolver on every
request in the chain and the delegate answers null for anything that is
not an authorization request. Recording the return target on those calls
cleared it again on the next request without a returnTo parameter — the
provider callback included, which this filter processes before login
succeeds. The success handler therefore always found an empty session
attribute and fell back to the default target, so returnTo never worked.
Guard the write on a non-null authorization request. As a side effect,
anonymous API requests no longer allocate a session via getSession().
Signed-off-by: Gal Eyal <gal.e@popai.health>
* 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>
The shared RedisTemplate uses GenericJackson2JsonRedisSerializer with
the application ObjectMapper, which embeds no type information, so
stored DeviceCodeData deserializes as a LinkedHashMap. The typed casts
in pollToken and authorizeDeviceCode then throw ClassCastException on
every call, making the whole device authorization flow unusable
(every poll returns 500).
Convert the raw value with ObjectMapper.convertValue instead of
casting; this reads both the current untyped map format and any typed
format, so no stored-data migration is needed. Adds bean setters to
DeviceCodeData for map conversion and regression tests that feed the
service exactly what Redis returns in production (untyped maps).
Fixes#604
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Gal Eyal <gal.e@popai.health>
* fix(publish): delete review tasks of any status when replacing a version
Re-uploading a rejected version under the same version number returned
HTTP 500. deleteReplaceableVersionArtifacts only removed a PENDING review
task, but a rejected version owns a REJECTED one; that row kept a foreign
key on the skill_version, so the subsequent delete hit a constraint
violation that surfaced as a 500.
Delete every review task attached to the version instead.
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
* test(publish): drop the spring-test dependency from the new test
skillhub-domain has no spring-test on its test classpath, so
ReflectionTestUtils does not resolve there. Use plain JDK reflection for
setting the generated id and invoking the private method.
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
* fix(publish): constrain rejected version replacement
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* test(publish): verify replaced review is deleted
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* test(e2e): use generated API response types
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
---------
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
The @Async rebuildSkill fix relied on a fresh thread giving a clean
transaction boundary. But skillhubEventExecutor uses CallerRunsPolicy:
under saturation the rejected task runs on the caller (request) thread,
back inside the afterCommit synchronization phase — the original failure
context where the @Transactional index write is silently dropped.
Mark SearchIndexService.index as REQUIRES_NEW so it always suspends any
lingering post-commit synchronization and commits in its own transaction,
independent of whether the async dispatch actually happened.
Add regression tests: detach removes the label keyword, and a synchronous
rebuild inside the afterCommit phase still persists the document (fails
without REQUIRES_NEW).
Signed-off-by: shychee <shychee96@gmail.com>
