From b0e19af3ed9976a952fc97072c85bb69f194bf5b Mon Sep 17 00:00:00 2001 From: yun-zhi-ztl <66589705+yun-zhi-ztl@users.noreply.github.com> Date: Tue, 17 Mar 2026 15:26:12 +0800 Subject: [PATCH] fix: harden hidden skill visibility and local dev restart flow (#62) * fix: hide hidden skills from regular viewers * fix: avoid dashboard preview crash after registration * fix: restrict skill hiding to super admins * chore: remove dev process script * fix: hide hidden skills from slug resolution --- Makefile | 2 +- scripts/dev-process.sh | 156 ++++++++++++++++++ scripts/dev_process.py | 144 ---------------- .../admin/AdminSkillController.java | 4 +- .../admin/AdminSkillReportController.java | 11 +- .../service/SkillSearchAppService.java | 7 +- .../admin/AdminSkillControllerTest.java | 18 +- .../admin/AdminSkillReportControllerTest.java | 24 ++- .../service/SkillSearchAppServiceTest.java | 39 ++++- .../service/SkillSlugResolutionService.java | 2 +- .../SkillSlugResolutionServiceTest.java | 10 ++ web/src/pages/dashboard-preview.test.ts | 8 + web/src/pages/dashboard-preview.ts | 7 +- web/src/pages/dashboard.tsx | 3 +- web/src/pages/skill-detail.tsx | 21 ++- 15 files changed, 286 insertions(+), 170 deletions(-) create mode 100755 scripts/dev-process.sh delete mode 100644 scripts/dev_process.py diff --git a/Makefile b/Makefile index be760c80..2f77d71c 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ DEV_API_URL := http://localhost:8080 STAGING_API_URL := http://localhost:8080 STAGING_WEB_URL := http://localhost STAGING_SERVER_IMAGE := skillhub-server:staging -DEV_PROCESS := python3 scripts/dev_process.py +DEV_PROCESS := bash scripts/dev-process.sh DEV_SERVER_PREPARE := true DEV_SERVER_CMD := ./scripts/run-dev-app.sh BACKEND_TEST_JAVA_OPTIONS ?= -XX:+EnableDynamicAgentLoading diff --git a/scripts/dev-process.sh b/scripts/dev-process.sh new file mode 100755 index 00000000..b5d2ebeb --- /dev/null +++ b/scripts/dev-process.sh @@ -0,0 +1,156 @@ +#!/usr/bin/env bash + +set -euo pipefail + +usage() { + echo "Usage:" >&2 + echo " $0 status --pid-file " >&2 + echo " $0 stop --pid-file " >&2 + echo " $0 start --pid-file --log-file --cwd -- " >&2 + exit 2 +} + +require_value() { + local flag="$1" + local value="${2:-}" + if [[ -z "$value" ]]; then + echo "Missing value for $flag" >&2 + usage + fi +} + +resolve_path() { + local path="$1" + if [[ "$path" = /* ]]; then + printf '%s\n' "$path" + else + printf '%s/%s\n' "$(pwd)" "$path" + fi +} + +is_running() { + local pid_file="$1" + [[ -f "$pid_file" ]] || return 1 + + local pid + pid="$(cat "$pid_file" 2>/dev/null || true)" + [[ "$pid" =~ ^[0-9]+$ ]] || return 1 + + if kill -0 "$pid" 2>/dev/null; then + return 0 + fi + + rm -f "$pid_file" + return 1 +} + +wait_for_exit() { + local pid="$1" + for _ in $(seq 1 50); do + if ! kill -0 "$pid" 2>/dev/null; then + return 0 + fi + sleep 0.1 + done + return 1 +} + +cmd="${1:-}" +[[ -n "$cmd" ]] || usage +shift || true + +pid_file="" +log_file="" +cwd="" + +case "$cmd" in + status|stop) + while [[ $# -gt 0 ]]; do + case "$1" in + --pid-file) + require_value "$1" "${2:-}" + pid_file="$2" + shift 2 + ;; + *) + usage + ;; + esac + done + [[ -n "$pid_file" ]] || usage + ;; + start) + while [[ $# -gt 0 ]]; do + case "$1" in + --pid-file) + require_value "$1" "${2:-}" + pid_file="$2" + shift 2 + ;; + --log-file) + require_value "$1" "${2:-}" + log_file="$2" + shift 2 + ;; + --cwd) + require_value "$1" "${2:-}" + cwd="$2" + shift 2 + ;; + --) + shift + break + ;; + *) + usage + ;; + esac + done + [[ -n "$pid_file" && -n "$log_file" && -n "$cwd" && $# -gt 0 ]] || usage + ;; + *) + usage + ;; +esac + +case "$cmd" in + status) + is_running "$pid_file" + ;; + stop) + if ! is_running "$pid_file"; then + rm -f "$pid_file" + exit 0 + fi + + pid="$(cat "$pid_file")" + kill "$pid" 2>/dev/null || true + if ! wait_for_exit "$pid"; then + kill -9 "$pid" 2>/dev/null || true + wait_for_exit "$pid" || true + fi + rm -f "$pid_file" + ;; + start) + pid_file="$(resolve_path "$pid_file")" + log_file="$(resolve_path "$log_file")" + cwd="$(resolve_path "$cwd")" + mkdir -p "$(dirname "$pid_file")" "$(dirname "$log_file")" + if is_running "$pid_file"; then + echo "Process already running with PID $(cat "$pid_file")" >&2 + exit 1 + fi + + ( + cd "$cwd" + if command -v setsid >/dev/null 2>&1; then + setsid "$@" >>"$log_file" 2>&1 < /dev/null & + else + nohup "$@" >>"$log_file" 2>&1 < /dev/null & + fi + child_pid=$! + disown "$child_pid" 2>/dev/null || true + echo "$child_pid" >"$pid_file" + ) + ;; +esac diff --git a/scripts/dev_process.py b/scripts/dev_process.py deleted file mode 100644 index d0450410..00000000 --- a/scripts/dev_process.py +++ /dev/null @@ -1,144 +0,0 @@ -#!/usr/bin/env python3 - -import argparse -import os -import signal -import subprocess -import sys -import time -from pathlib import Path -from typing import Optional - - -def is_running(pid: int) -> bool: - try: - os.kill(pid, 0) - except OSError: - return False - return True - - -def read_pid(pid_file: Path) -> Optional[int]: - if not pid_file.exists(): - return None - content = pid_file.read_text(encoding="utf-8").strip() - if not content: - return None - try: - return int(content) - except ValueError: - return None - - -def write_pid(pid_file: Path, pid: int) -> None: - pid_file.parent.mkdir(parents=True, exist_ok=True) - pid_file.write_text(f"{pid}\n", encoding="utf-8") - - -def start_process(args: argparse.Namespace) -> int: - pid_file = Path(args.pid_file) - log_file = Path(args.log_file) - cwd = Path(args.cwd) - existing_pid = read_pid(pid_file) - if existing_pid and is_running(existing_pid): - print(existing_pid) - return 0 - - pid_file.unlink(missing_ok=True) - log_file.parent.mkdir(parents=True, exist_ok=True) - - command = list(args.command) - if command and command[0] == "--": - command = command[1:] - - with log_file.open("ab") as log_handle, open(os.devnull, "rb") as devnull: - process = subprocess.Popen( - command, - cwd=cwd, - stdin=devnull, - stdout=log_handle, - stderr=subprocess.STDOUT, - start_new_session=True, - ) - - time.sleep(0.2) - if process.poll() is not None: - pid_file.unlink(missing_ok=True) - return process.returncode or 1 - - write_pid(pid_file, process.pid) - print(process.pid) - return 0 - - -def stop_process(args: argparse.Namespace) -> int: - pid_file = Path(args.pid_file) - pid = read_pid(pid_file) - if not pid: - return 0 - - if not is_running(pid): - pid_file.unlink(missing_ok=True) - return 0 - - os.kill(pid, signal.SIGTERM) - deadline = time.time() + args.timeout - while time.time() < deadline: - if not is_running(pid): - pid_file.unlink(missing_ok=True) - return 0 - time.sleep(0.2) - - os.kill(pid, signal.SIGKILL) - pid_file.unlink(missing_ok=True) - return 0 - - -def status_process(args: argparse.Namespace) -> int: - pid_file = Path(args.pid_file) - pid = read_pid(pid_file) - if not pid: - return 1 - if not is_running(pid): - pid_file.unlink(missing_ok=True) - return 1 - print(pid) - return 0 - - -def build_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Manage detached dev processes.") - subparsers = parser.add_subparsers(dest="action", required=True) - - start_parser = subparsers.add_parser("start") - start_parser.add_argument("--pid-file", required=True) - start_parser.add_argument("--log-file", required=True) - start_parser.add_argument("--cwd", required=True) - start_parser.add_argument("command", nargs=argparse.REMAINDER) - - stop_parser = subparsers.add_parser("stop") - stop_parser.add_argument("--pid-file", required=True) - stop_parser.add_argument("--timeout", type=float, default=10.0) - - status_parser = subparsers.add_parser("status") - status_parser.add_argument("--pid-file", required=True) - return parser - - -def main() -> int: - parser = build_parser() - args = parser.parse_args() - - if args.action == "start": - if not args.command: - parser.error("start requires a command") - return start_process(args) - if args.action == "stop": - return stop_process(args) - if args.action == "status": - return status_process(args) - return 1 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillController.java index 25e3d053..708cfdb8 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillController.java @@ -29,7 +29,7 @@ public class AdminSkillController extends BaseApiController { } @PostMapping("/{skillId}/hide") - @PreAuthorize("hasAnyRole('SKILL_ADMIN', 'SUPER_ADMIN')") + @PreAuthorize("hasRole('SUPER_ADMIN')") public ApiResponse hideSkill(@PathVariable Long skillId, @RequestBody(required = false) AdminSkillActionRequest request, @AuthenticationPrincipal PlatformPrincipal principal, @@ -45,7 +45,7 @@ public class AdminSkillController extends BaseApiController { } @PostMapping("/{skillId}/unhide") - @PreAuthorize("hasAnyRole('SKILL_ADMIN', 'SUPER_ADMIN')") + @PreAuthorize("hasRole('SUPER_ADMIN')") public ApiResponse unhideSkill(@PathVariable Long skillId, @AuthenticationPrincipal PlatformPrincipal principal, HttpServletRequest httpRequest) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReportController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReportController.java index 0442a543..232ed8c0 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReportController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReportController.java @@ -4,6 +4,7 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.controller.BaseApiController; import com.iflytek.skillhub.domain.report.SkillReportDisposition; import com.iflytek.skillhub.domain.report.SkillReportService; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.dto.AdminSkillReportActionRequest; import com.iflytek.skillhub.dto.AdminSkillReportSummaryResponse; import com.iflytek.skillhub.dto.ApiResponse; @@ -52,12 +53,16 @@ public class AdminSkillReportController extends BaseApiController { @RequestBody(required = false) AdminSkillReportActionRequest request, @AuthenticationPrincipal PlatformPrincipal principal, HttpServletRequest httpRequest) { + SkillReportDisposition disposition = request != null && request.disposition() != null + ? SkillReportDisposition.valueOf(request.disposition().trim().toUpperCase()) + : SkillReportDisposition.RESOLVE_ONLY; + if (disposition == SkillReportDisposition.RESOLVE_AND_HIDE && !principal.platformRoles().contains("SUPER_ADMIN")) { + throw new DomainForbiddenException("error.skill.lifecycle.noPermission"); + } var report = skillReportService.resolveReport( reportId, principal.userId(), - request != null && request.disposition() != null - ? SkillReportDisposition.valueOf(request.disposition().trim().toUpperCase()) - : SkillReportDisposition.RESOLVE_ONLY, + disposition, request != null ? request.comment() : null, httpRequest.getRemoteAddr(), httpRequest.getHeader("User-Agent") diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java index 1e5da1c5..4a7cbf32 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java @@ -7,6 +7,7 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceService; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.VisibilityChecker; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; @@ -31,18 +32,21 @@ public class SkillSearchAppService { private final NamespaceRepository namespaceRepository; private final SkillVersionRepository skillVersionRepository; private final NamespaceService namespaceService; + private final VisibilityChecker visibilityChecker; public SkillSearchAppService( SearchQueryService searchQueryService, SkillRepository skillRepository, NamespaceRepository namespaceRepository, SkillVersionRepository skillVersionRepository, - NamespaceService namespaceService) { + NamespaceService namespaceService, + VisibilityChecker visibilityChecker) { this.searchQueryService = searchQueryService; this.skillRepository = skillRepository; this.namespaceRepository = namespaceRepository; this.skillVersionRepository = skillVersionRepository; this.namespaceService = namespaceService; + this.visibilityChecker = visibilityChecker; } public record SearchResponse( @@ -171,6 +175,7 @@ public class SkillSearchAppService { return skillIds.stream() .map(skillsById::get) .filter(java.util.Objects::nonNull) + .filter(skill -> visibilityChecker.canAccess(skill, userId, userNsRoles != null ? userNsRoles : Map.of())) .filter(skill -> namespaceVisible(skill.getNamespaceId(), namespacesById, userId, userNsRoles)) .map(skill -> toSummaryResponse(skill, versionsById, namespaceSlugsById)) .toList(); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillControllerTest.java index 8901d105..2a6215b3 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillControllerTest.java @@ -51,8 +51,8 @@ class AdminSkillControllerTest { given(skillGovernanceService.hideSkill(org.mockito.ArgumentMatchers.eq(10L), org.mockito.ArgumentMatchers.eq("admin"), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.eq("policy"))) .willReturn(skill); - PlatformPrincipal principal = new PlatformPrincipal("admin", "admin", "a@example.com", "", "github", Set.of("SKILL_ADMIN")); - var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN"))); + PlatformPrincipal principal = new PlatformPrincipal("admin", "admin", "a@example.com", "", "github", Set.of("SUPER_ADMIN")); + var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN"))); mockMvc.perform(post("/api/v1/admin/skills/10/hide") .with(authentication(auth)) @@ -100,4 +100,18 @@ class AdminSkillControllerTest { .andExpect(status().isForbidden()) .andExpect(jsonPath("$.code").value(403)); } + + @Test + void hideSkill_withSkillAdminRole_returns403() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal("admin", "admin", "a@example.com", "", "github", Set.of("SKILL_ADMIN")); + var auth = new UsernamePasswordAuthenticationToken(principal, null, List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN"))); + + mockMvc.perform(post("/api/v1/admin/skills/10/hide") + .with(authentication(auth)) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"reason\":\"policy\"}")) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillReportControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillReportControllerTest.java index 1f3cf509..0ff84bb4 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillReportControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/AdminSkillReportControllerTest.java @@ -95,7 +95,7 @@ class AdminSkillReportControllerTest { report.setStatus(com.iflytek.skillhub.domain.report.SkillReportStatus.RESOLVED); when(skillReportService.resolveReport( org.mockito.ArgumentMatchers.eq(99L), - org.mockito.ArgumentMatchers.eq("admin"), + org.mockito.ArgumentMatchers.eq("super-admin"), org.mockito.ArgumentMatchers.eq(SkillReportDisposition.RESOLVE_AND_HIDE), org.mockito.ArgumentMatchers.eq("handled"), org.mockito.ArgumentMatchers.any(), @@ -103,7 +103,7 @@ class AdminSkillReportControllerTest { .thenReturn(report); mockMvc.perform(post("/api/v1/admin/skill-reports/99/resolve") - .with(authentication(adminAuth())) + .with(authentication(superAdminAuth())) .with(csrf()) .contentType(APPLICATION_JSON) .content("{\"comment\":\"handled\",\"disposition\":\"RESOLVE_AND_HIDE\"}")) @@ -112,6 +112,17 @@ class AdminSkillReportControllerTest { .andExpect(jsonPath("$.data.status").value("RESOLVED")); } + @Test + void resolveReport_withHideDispositionAndSkillAdmin_returns403() throws Exception { + mockMvc.perform(post("/api/v1/admin/skill-reports/99/resolve") + .with(authentication(adminAuth())) + .with(csrf()) + .contentType(APPLICATION_JSON) + .content("{\"comment\":\"handled\",\"disposition\":\"RESOLVE_AND_HIDE\"}")) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } + @Test void listReports_withAuditorRole_returns403() throws Exception { PlatformPrincipal principal = new PlatformPrincipal( @@ -136,4 +147,13 @@ class AdminSkillReportControllerTest { principal, null, List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN")) ); } + + private UsernamePasswordAuthenticationToken superAdminAuth() { + PlatformPrincipal principal = new PlatformPrincipal( + "super-admin", "super-admin", "admin@example.com", "", "github", Set.of("SUPER_ADMIN") + ); + return new UsernamePasswordAuthenticationToken( + principal, null, List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN")) + ); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java index 755e04b1..e739ce30 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java @@ -7,6 +7,7 @@ import com.iflytek.skillhub.domain.namespace.NamespaceStatus; import com.iflytek.skillhub.domain.namespace.NamespaceService; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.VisibilityChecker; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.search.SearchQueryService; @@ -47,7 +48,14 @@ class SkillSearchAppServiceTest { @BeforeEach void setUp() { - service = new SkillSearchAppService(searchQueryService, skillRepository, namespaceRepository, skillVersionRepository, namespaceService); + service = new SkillSearchAppService( + searchQueryService, + skillRepository, + namespaceRepository, + skillVersionRepository, + namespaceService, + new VisibilityChecker() + ); } @Test @@ -74,8 +82,10 @@ class SkillSearchAppServiceTest { void search_shouldFillVisiblePageAcrossArchivedNamespaceResults() { Skill archivedSkill = new Skill(1L, "archived-skill", "owner-1", SkillVisibility.PUBLIC); setField(archivedSkill, "id", 10L); + archivedSkill.setLatestVersionId(110L); Skill visibleSkill = new Skill(2L, "visible-skill", "owner-1", SkillVisibility.PUBLIC); setField(visibleSkill, "id", 11L); + visibleSkill.setLatestVersionId(111L); Namespace archivedNamespace = new Namespace("archived-team", "Archived Team", "owner-1"); setField(archivedNamespace, "id", 1L); @@ -114,6 +124,33 @@ class SkillSearchAppServiceTest { ); } + @Test + void search_shouldExcludeHiddenSkillsForRegularUsers() { + Skill visibleSkill = new Skill(1L, "visible-skill", "owner-1", SkillVisibility.PUBLIC); + setField(visibleSkill, "id", 10L); + visibleSkill.setLatestVersionId(101L); + + Skill hiddenSkill = new Skill(1L, "hidden-skill", "owner-2", SkillVisibility.PUBLIC); + setField(hiddenSkill, "id", 11L); + hiddenSkill.setLatestVersionId(102L); + hiddenSkill.setHidden(true); + + Namespace namespace = new Namespace("team-a", "Team A", "owner-1"); + setField(namespace, "id", 1L); + namespace.setStatus(NamespaceStatus.ACTIVE); + + when(searchQueryService.search(org.mockito.ArgumentMatchers.any())) + .thenReturn(new SearchResult(List.of(10L, 11L), 2, 0, 20)); + when(skillRepository.findByIdIn(List.of(10L, 11L))).thenReturn(List.of(visibleSkill, hiddenSkill)); + when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace)); + + SkillSearchAppService.SearchResponse response = service.search("skill", null, "newest", 0, 20, "user-9", Map.of()); + + assertEquals(1, response.items().size()); + assertEquals("visible-skill", response.items().getFirst().slug()); + assertEquals(1, response.total()); + } + private void setField(Object target, String fieldName, Object value) { try { java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionService.java index d7f9d1bf..e4521da3 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionService.java @@ -32,7 +32,7 @@ public class SkillSlugResolutionService { ? Optional.empty() : skills.stream().filter(skill -> currentUserId.equals(skill.getOwnerId())).findFirst(); Optional publishedSkill = skills.stream() - .filter(skill -> skill.getLatestVersionId() != null) + .filter(skill -> skill.getLatestVersionId() != null && !skill.isHidden()) .findFirst(); if (preference == Preference.CURRENT_USER) { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionServiceTest.java index 8fdbdac1..a9233c89 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionServiceTest.java @@ -58,6 +58,16 @@ class SkillSlugResolutionServiceTest { service.resolve(1L, "demo", null, SkillSlugResolutionService.Preference.CURRENT_USER)); } + @Test + void throwsWhenOnlyPublishedSkillIsHiddenFromCurrentUser() throws Exception { + Skill hiddenPublishedSkill = createSkill(4L, "demo", "user-2", 44L); + hiddenPublishedSkill.setHidden(true); + when(skillRepository.findByNamespaceIdAndSlug(1L, "demo")).thenReturn(List.of(hiddenPublishedSkill)); + + assertThrows(DomainBadRequestException.class, () -> + service.resolve(1L, "demo", "user-9", SkillSlugResolutionService.Preference.CURRENT_USER)); + } + private Skill createSkill(Long id, String slug, String ownerId, Long latestVersionId) throws Exception { Skill skill = new Skill(1L, slug, ownerId, SkillVisibility.PUBLIC); Field idField = Skill.class.getDeclaredField("id"); diff --git a/web/src/pages/dashboard-preview.test.ts b/web/src/pages/dashboard-preview.test.ts index 0624595f..4b71e656 100644 --- a/web/src/pages/dashboard-preview.test.ts +++ b/web/src/pages/dashboard-preview.test.ts @@ -25,4 +25,12 @@ describe('limitPreviewItems', () => { remainingCount: 1, }) }) + + it('returns an empty preview instead of throwing when input is not an array', () => { + expect(limitPreviewItems({ items: ['a', 'b'] } as never, 3)).toEqual({ + items: [], + hasMore: false, + remainingCount: 0, + }) + }) }) diff --git a/web/src/pages/dashboard-preview.ts b/web/src/pages/dashboard-preview.ts index 66fd18c9..8cf9c8e2 100644 --- a/web/src/pages/dashboard-preview.ts +++ b/web/src/pages/dashboard-preview.ts @@ -1,10 +1,11 @@ -export function limitPreviewItems(items: T[], limit: number): { +export function limitPreviewItems(items: T[] | null | undefined | unknown, limit: number): { items: T[] hasMore: boolean remainingCount: number } { - const visibleItems = items.slice(0, limit) - const remainingCount = Math.max(items.length - visibleItems.length, 0) + const normalizedItems: T[] = Array.isArray(items) ? (items as T[]) : [] + const visibleItems = normalizedItems.slice(0, limit) + const remainingCount = Math.max(normalizedItems.length - visibleItems.length, 0) return { items: visibleItems, diff --git a/web/src/pages/dashboard.tsx b/web/src/pages/dashboard.tsx index 082e378b..896ec209 100644 --- a/web/src/pages/dashboard.tsx +++ b/web/src/pages/dashboard.tsx @@ -1,6 +1,7 @@ import { Link } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useAuth } from '@/features/auth/use-auth' +import type { SkillSummary } from '@/api/types' import { useMySkills } from '@/shared/hooks/use-skill-queries' import { TokenList } from '@/features/token/token-list' import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card' @@ -14,7 +15,7 @@ export function DashboardPage() { const { user, hasRole } = useAuth() const governanceVisible = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN') const { data: skillPage, isLoading: isLoadingSkills } = useMySkills({ page: 0, size: skillPreviewPageSize }) - const skillPreview = limitPreviewItems(skillPage?.items ?? [], DASHBOARD_PREVIEW_LIMIT) + const skillPreview = limitPreviewItems(skillPage?.items ?? [], DASHBOARD_PREVIEW_LIMIT) return (
diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index db350b69..9d1582d7 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -107,6 +107,7 @@ export function SkillDetailPage() { const { data: diffSourceReadme } = useSkillReadme(namespace, slug, diffSourceVersion ?? undefined, diffSourceDocumentationPath) const { data: diffCompareReadme } = useSkillReadme(namespace, slug, diffCompareVersion ?? undefined, diffCompareDocumentationPath) const governanceVisible = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN') + const canHideSkill = hasRole('SUPER_ADMIN') const isPendingPreview = skill?.viewingVersionStatus === 'PENDING_REVIEW' const canInteract = skill?.canInteract ?? true const canReport = skill?.canReport ?? true @@ -736,15 +737,17 @@ export function SkillDetailPage() {
{t('skillDetail.governance')}
- {!skill.hidden ? ( - - ) : ( - - )} + {canHideSkill ? ( + !skill.hidden ? ( + + ) : ( + + ) + ) : null} {selectedVersionEntry && (