scriptoriumadmin/skillhub
1
0
Fork 0
mirror of https://github.com/iflytek/skillhub.git synced 2026-10-07 02:57:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(scanner): harden scanner HTTP contract

Browse source 
Signed-off-by: dongmucat <1127093059@qq.com>
This commit is contained in:
dongmucat 2026-09-18 10:58:10 +08:00
parent 475c49702c
commit 29f5c4cf86
13 changed files with 289 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
server/skillhub-app/src/main/java/com/iflytek/skillhub/config/SkillScannerConfig.java
View file

@ -91,6 +91,8 @@ public class SkillScannerConfig {
analyzers.isBehavioral(),
analyzers.isLlm(),
analyzers.getLlmProvider(),
analyzers.getLlmConsensusRuns(),
properties.getPolicy().getPreset(),
analyzers.isMeta(),
analyzers.isAiDefense(),
analyzers.getAiDefenseApiKey(),

26
server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SecurityAuditControllerTest.java
View file

@ -40,7 +40,10 @@ import static org.springframework.security.test.web.servlet.request.SecurityMock
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
@SpringBootTest
@AutoConfigureMockMvc
@ -128,6 +131,29 @@ class SecurityAuditControllerTest {
.andExpect(jsonPath("$.data[0].findings[0].ruleId").value("STATIC-001"));
}
@Test
void getSecurityAudit_doesNotExposeUnmaskedCanaryFromStoredFinding() throws Exception {
SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER);
setField(audit, "id", 8L);
audit.setScanId("scan-masked");
audit.setVerdict(SecurityVerdict.DANGEROUS);
audit.setIsSafe(false);
audit.setMaxSeverity("HIGH");
audit.setFindings("""
[{"ruleId":"TOKEN-001","severity":"HIGH","category":"secrets","title":"Token detected","message":"<redacted>","filePath":"SKILL.md","lineNumber":4,"codeSnippet":"token=<redacted>"}]
""".trim());
given(skillVersionRepository.findById(42L)).willReturn(java.util.Optional.of(skillVersion(42L, 8L)));
given(skillRepository.findById(8L)).willReturn(java.util.Optional.of(skill(8L, "reviewer-1")));
given(securityAuditRepository.findLatestActiveByVersionId(42L)).willReturn(List.of(audit));
mockMvc.perform(get("/api/v1/skills/8/versions/42/security-audit")
.with(auth("reviewer-1"))
.requestAttr("userNsRoles", Map.of(5L, NamespaceRole.ADMIN)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.data[0].findings[0].message").value("<redacted>"))
.andExpect(content().string(not(containsString("ghp_012345678901234567890123456789012345"))));
}
@Test
void getSecurityAudit_returnsEmptyListWhenAuditMissing() throws Exception {
given(skillVersionRepository.findById(42L)).willReturn(java.util.Optional.of(skillVersion(42L, 8L)));

24
server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/security/SecurityScanServiceTest.java
View file

@ -18,6 +18,7 @@ import org.springframework.transaction.support.TransactionSynchronizationManager
import java.lang.reflect.Field;
import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
@ -404,6 +405,29 @@ class SecurityScanServiceTest {
verify(skillVersionRepository).save(version);
}
@Test
void processScanResult_persistsScannerMaskedFindingWithoutOriginalCanary() {
SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-masked");
SkillVersion version = new SkillVersion(8L, "1.0.0", "publisher-1");
version.setStatus(SkillVersionStatus.SCANNING);
given(auditRepository.findByTaskId("task-masked")).willReturn(Optional.of(audit));
given(auditRepository.findLatestActiveByVersionIdAndScannerType(42L, ScannerType.SKILL_SCANNER))
.willReturn(Optional.of(audit));
given(skillVersionRepository.findById(42L)).willReturn(Optional.of(version));
String canary = "ghp_012345678901234567890123456789012345";
SecurityFinding maskedFinding = new SecurityFinding(
"TOKEN-001", "HIGH", "secrets", "Token detected", "<redacted>",
"SKILL.md", 4, "token=<redacted>", "Rotate token", "static", Map.of());
service.processScanResult(
"task-masked", 42L, ScannerType.SKILL_SCANNER,
new SecurityScanResponse("scan-masked", SecurityVerdict.DANGEROUS, 1, "HIGH",
List.of(maskedFinding), 0.2));
assertThat(audit.getFindings()).contains("<redacted>").doesNotContain(canary);
}
@Test
void processScanResult_forStaleAttemptDoesNotCompleteCurrentAttempt() throws Exception {
SecurityAudit stale = new SecurityAudit(42L, ScannerType.SKILL_SCANNER, "task-stale");

6
server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/http/HttpClient.java
View file

@ -7,7 +7,11 @@ public interface HttpClient {
<T> T get(String uri, Class<T> responseType);
<T> T post(String uri, Object body, Class<T> responseType);
default <T> T post(String uri, Object body, Class<T> responseType) {
return post(uri, body, new HttpHeaders(), responseType);
}
<T> T post(String uri, Object body, HttpHeaders headers, Class<T> responseType);
<T> T postMultipart(String uri, MultiValueMap<String, Object> parts, Class<T> responseType);

10
server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/http/HttpClientException.java
View file

@ -6,9 +6,11 @@ public class HttpClientException extends RuntimeException {
private final String responseBody;
public HttpClientException(int statusCode, String responseBody) {
super("HTTP " + statusCode + ": " + responseBody);
super(responseBody == null || responseBody.isBlank()
? "HTTP " + statusCode
: "HTTP " + statusCode + ": " + bounded(responseBody));
this.statusCode = statusCode;
this.responseBody = responseBody;
this.responseBody = responseBody == null ? null : bounded(responseBody);
}
public HttpClientException(String message, Throwable cause) {
@ -33,4 +35,8 @@ public class HttpClientException extends RuntimeException {
String message = root.getMessage();
return root.getClass().getSimpleName() + (message == null || message.isBlank() ? "" : ": " + message);
}
private static String bounded(String body) {
return body.length() <= 2048 ? body : body.substring(0, 2048) + "...[truncated]";
}
}

6
server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/http/WebClientHttpClient.java
View file

@ -39,10 +39,16 @@ public class WebClientHttpClient implements HttpClient {
@Override
public <T> T post(String uri, Object body, Class<T> responseType) {
return post(uri, body, new HttpHeaders(), responseType);
}
@Override
public <T> T post(String uri, Object body, HttpHeaders headers, Class<T> responseType) {
log.debug("POST {}", uri);
try {
return webClient.post()
.uri(uri)
.headers(httpHeaders -> httpHeaders.addAll(headers))
.contentType(MediaType.APPLICATION_JSON)
.bodyValue(body)
.retrieve()

28
server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/scanner/ScanOptions.java
View file

@ -4,6 +4,8 @@ public record ScanOptions(
boolean useBehavioral,
boolean useLlm,
String llmProvider,
int llmConsensusRuns,
String policyPreset,
boolean enableMeta,
boolean useAidefense,
String aidefenseApiKey,
@ -11,7 +13,31 @@ public record ScanOptions(
boolean useTrigger
) {
public ScanOptions {
if (llmConsensusRuns < 1) {
throw new IllegalArgumentException("llmConsensusRuns must be at least 1");
}
if (!"strict".equals(policyPreset)
&& !"balanced".equals(policyPreset)
&& !"permissive".equals(policyPreset)) {
throw new IllegalArgumentException("policyPreset must be strict, balanced, or permissive");
}
}
/** Backward-compatible constructor for callers that do not configure the new scanner options. */
public ScanOptions(boolean useBehavioral,
boolean useLlm,
String llmProvider,
boolean enableMeta,
boolean useAidefense,
String aidefenseApiKey,
boolean useVirusTotal,
boolean useTrigger) {
this(useBehavioral, useLlm, llmProvider, 1, "balanced", enableMeta,
useAidefense, aidefenseApiKey, useVirusTotal, useTrigger);
}
public static ScanOptions disabled() {
return new ScanOptions(false, false, "anthropic", false, false, "", false, false);
return new ScanOptions(false, false, "anthropic", 1, "balanced", false, false, "", false, false);
}
}

43
server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/scanner/SkillScannerService.java
View file

@ -38,25 +38,26 @@ public class SkillScannerService {
Map<String, Object> body = buildScanRequestBody(skillDirectory, options);
try {
return httpClient.post(uri, body, SkillScannerApiResponse.class);
return httpClient.post(uri, body, buildScannerHeaders(options), SkillScannerApiResponse.class);
} catch (HttpClientException e) {
log.error("Scanner API error: status={}, body={}", e.getStatusCode(), summarizeResponseBody(e.getResponseBody()));
throw e;
log.error("Scanner API error: status={}, operation=scanDirectory", e.getStatusCode());
throw sanitizedException(e);
}
}
public SkillScannerApiResponse scanUpload(Path skillPackagePath, ScanOptions options) {
String uri = buildUploadUri(options);
log.info("Uploading skill package to scanner: {}", sanitizeUri(uri));
log.info("Uploading skill package to scanner: {}", uri);
MultiValueMap<String, Object> parts = new LinkedMultiValueMap<>();
parts.add("file", new FileSystemResource(skillPackagePath));
addScannerOptionsParts(parts, options);
HttpHeaders headers = buildScannerHeaders(options);
try {
return httpClient.postMultipart(uri, parts, headers, SkillScannerApiResponse.class);
} catch (HttpClientException e) {
log.error("Scanner API error: status={}, body={}", e.getStatusCode(), summarizeResponseBody(e.getResponseBody()));
throw e;
log.error("Scanner API error: status={}, operation=scanUpload", e.getStatusCode());
throw sanitizedException(e);
}
}
@ -70,6 +71,8 @@ public class SkillScannerService {
body.put("use_behavioral", options.useBehavioral());
body.put("use_llm", options.useLlm());
body.put("llm_provider", options.llmProvider());
body.put("llm_consensus_runs", options.llmConsensusRuns());
body.put("policy", options.policyPreset());
body.put("enable_meta", options.enableMeta());
body.put("use_aidefense", options.useAidefense());
if (options.useAidefense() && !options.aidefenseApiKey().isEmpty()) {
@ -80,6 +83,18 @@ public class SkillScannerService {
return body;
}
private void addScannerOptionsParts(MultiValueMap<String, Object> parts, ScanOptions options) {
parts.add("use_behavioral", Boolean.toString(options.useBehavioral()));
parts.add("use_llm", Boolean.toString(options.useLlm()));
parts.add("llm_provider", options.llmProvider());
parts.add("llm_consensus_runs", Integer.toString(options.llmConsensusRuns()));
parts.add("policy", options.policyPreset());
parts.add("enable_meta", Boolean.toString(options.enableMeta()));
parts.add("use_aidefense", Boolean.toString(options.useAidefense()));
parts.add("use_virustotal", Boolean.toString(options.useVirusTotal()));
parts.add("use_trigger", Boolean.toString(options.useTrigger()));
}
private String buildUploadUri(ScanOptions options) {
StringBuilder uri = new StringBuilder(baseUrl + scanPath);
uri.append("?use_behavioral=").append(options.useBehavioral());
@ -95,7 +110,7 @@ public class SkillScannerService {
private HttpHeaders buildScannerHeaders(ScanOptions options) {
HttpHeaders headers = new HttpHeaders();
if (options.useAidefense() && !options.aidefenseApiKey().isEmpty()) {
headers.add("X-AIDefense-Api-Key", options.aidefenseApiKey());
headers.add("X-AIDefense-Key", options.aidefenseApiKey());
}
return headers;
}
@ -116,15 +131,11 @@ public class SkillScannerService {
return normalized.endsWith("/") ? normalized.substring(0, normalized.length() - 1) : normalized;
}
private String summarizeResponseBody(String body) {
if (body == null || body.isBlank()) {
return "<empty>";
private HttpClientException sanitizedException(HttpClientException exception) {
if (exception.getStatusCode() > 0) {
return new HttpClientException(exception.getStatusCode(), null);
}
String singleLine = body.replaceAll("\\s+", " ").trim();
return singleLine.length() > 200 ? singleLine.substring(0, 200) + "...[truncated]" : singleLine;
}
private String sanitizeUri(String uri) {
return uri.replaceAll("([?&]aidefense_api_key=)[^&]+", "$1***");
Throwable cause = exception.getCause() == null ? exception : exception.getCause();
return new HttpClientException("Scanner API request failed", cause);
}
}

80
server/skillhub-infra/src/test/java/com/iflytek/skillhub/infra/http/WebClientHttpClientTest.java Normal file
View file

@ -0,0 +1,80 @@
package com.iflytek.skillhub.infra.http;
import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpServer;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.web.reactive.function.client.WebClient;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import static org.assertj.core.api.Assertions.assertThat;
class WebClientHttpClientTest {
private HttpServer server;
private AtomicReference<String> requestBody;
private AtomicReference<String> requestHeader;
@BeforeEach
void setUp() throws IOException {
requestBody = new AtomicReference<>();
requestHeader = new AtomicReference<>();
server = HttpServer.create(new InetSocketAddress("localhost", 0), 0);
server.createContext("/json", this::handleJson);
server.start();
}
@AfterEach
void tearDown() {
server.stop(0);
}
@Test
void postJson_sendsHeadersAndDecodesResponse() {
WebClientHttpClient client = new WebClientHttpClient(WebClient.builder().build());
@SuppressWarnings("unchecked")
Map<String, Object> response = (Map<String, Object>) client.post(
endpoint(),
Map.of("message", "hello"),
new org.springframework.http.HttpHeaders() {{ set("X-Test", "header-value"); }},
Map.class
);
assertThat(response).containsEntry("ok", true);
assertThat(requestHeader.get()).isEqualTo("header-value");
assertThat(requestBody.get()).contains("\"message\":\"hello\"");
}
@Test
void postJson_withoutHeaders_remainsSupported() {
WebClientHttpClient client = new WebClientHttpClient(WebClient.builder().build());
@SuppressWarnings("unchecked")
Map<String, Object> response = (Map<String, Object>) client.post(endpoint(), Map.of("message", "legacy"), Map.class);
assertThat(response).containsEntry("ok", true);
assertThat(requestHeader.get()).isNull();
}
private String endpoint() {
return "http://localhost:" + server.getAddress().getPort() + "/json";
}
private void handleJson(HttpExchange exchange) throws IOException {
requestBody.set(new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8));
requestHeader.set(exchange.getRequestHeaders().getFirst("X-Test"));
byte[] response = "{\"ok\":true}".getBytes(StandardCharsets.UTF_8);
exchange.getResponseHeaders().set("Content-Type", "application/json");
exchange.sendResponseHeaders(200, response.length);
try (var output = exchange.getResponseBody()) {
output.write(response);
}
}
}

33
server/skillhub-infra/src/test/java/com/iflytek/skillhub/infra/scanner/ScanOptionsTest.java Normal file
View file

@ -0,0 +1,33 @@
package com.iflytek.skillhub.infra.scanner;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
class ScanOptionsTest {
@Test
void disabled_usesSafeConsensusAndBalancedPolicyDefaults() {
ScanOptions options = ScanOptions.disabled();
assertThat(options.llmConsensusRuns()).isEqualTo(1);
assertThat(options.policyPreset()).isEqualTo("balanced");
}
@Test
void rejectsInvalidConsensusRuns() {
assertThatThrownBy(() -> new ScanOptions(
false, false, "anthropic", 0, "balanced", false, false, "", false, false))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("llmConsensusRuns");
}
@Test
void rejectsUnknownPolicyPreset() {
assertThatThrownBy(() -> new ScanOptions(
false, false, "anthropic", 1, "custom", false, false, "", false, false))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("policyPreset");
}
}

5
server/skillhub-infra/src/test/java/com/iflytek/skillhub/infra/scanner/SkillScannerAdapterTest.java
View file

@ -168,6 +168,11 @@ class SkillScannerAdapterTest {
throw new UnsupportedOperationException();
}
@Override
public <T> T post(String uri, Object body, HttpHeaders headers, Class<T> responseType) {
throw new UnsupportedOperationException();
}
@Override
public <T> T postMultipart(String uri, org.springframework.util.MultiValueMap<String, Object> parts,
Class<T> responseType) {

10
server/skillhub-infra/src/test/java/com/iflytek/skillhub/infra/scanner/SkillScannerLoggingTest.java
View file

@ -122,6 +122,11 @@ class SkillScannerLoggingTest {
throw new UnsupportedOperationException();
}
@Override
public <T> T post(String uri, Object body, HttpHeaders headers, Class<T> responseType) {
throw new UnsupportedOperationException();
}
@Override
public <T> T postMultipart(String uri, MultiValueMap<String, Object> parts, Class<T> responseType) {
throw new UnsupportedOperationException();
@ -165,6 +170,11 @@ class SkillScannerLoggingTest {
throw new UnsupportedOperationException();
}
@Override
public <T> T post(String uri, Object body, HttpHeaders headers, Class<T> responseType) {
throw new UnsupportedOperationException();
}
@Override
public <T> T postMultipart(String uri, MultiValueMap<String, Object> parts, Class<T> responseType) {
throw new UnsupportedOperationException();

37
server/skillhub-infra/src/test/java/com/iflytek/skillhub/infra/scanner/SkillScannerServiceTest.java
View file

@ -63,6 +63,8 @@ class SkillScannerServiceTest {
assertThat(body.get("skill_directory")).isEqualTo("/tmp/demo");
assertThat(body.get("use_behavioral")).isEqualTo(true);
assertThat(body.get("use_llm")).isEqualTo(false);
assertThat(body.get("llm_consensus_runs")).isEqualTo(1);
assertThat(body.get("policy")).isEqualTo("balanced");
}
@Test
@ -94,6 +96,8 @@ class SkillScannerServiceTest {
assertThat(httpClient.lastMultipartUri).contains("use_llm=true");
assertThat(httpClient.lastMultipartUri).contains("llm_provider=openai");
assertThat(httpClient.lastMultipartParts.getFirst("file")).isNotNull();
assertThat(httpClient.lastMultipartParts.getFirst("llm_consensus_runs")).isEqualTo("1");
assertThat(httpClient.lastMultipartParts.getFirst("policy")).isEqualTo("balanced");
}
@Test
@ -120,7 +124,28 @@ class SkillScannerServiceTest {
service.scanUpload(Path.of("/tmp/demo.zip"), options);
assertThat(httpClient.lastMultipartUri).doesNotContain("aidefense_api_key");
assertThat(httpClient.lastMultipartHeaders.getFirst("X-AIDefense-Api-Key")).isEqualTo("secret-key");
assertThat(httpClient.lastMultipartHeaders.getFirst("X-AIDefense-Key")).isEqualTo("secret-key");
}
@Test
void scanDirectory_keepsLegacyAidefenseBodyFieldAndSendsHeader() {
FakeHttpClient httpClient = new FakeHttpClient();
httpClient.postResponse = new SkillScannerApiResponse(
"scan-4", "test-skill", true, "LOW", 0, null, 0.5, "2026-03-22T07:00:00");
SkillScannerService service = new SkillScannerService(
httpClient, "http://scanner.test", "/scan-upload", "/health");
ScanOptions options = new ScanOptions(false, false, "anthropic", 3, "strict",
false, true, "secret-key", false, false);
service.scanDirectory("/tmp/demo", options);
@SuppressWarnings("unchecked")
Map<String, Object> body = (Map<String, Object>) httpClient.lastPostBody;
assertThat(body.get("aidefense_api_key")).isEqualTo("secret-key");
assertThat(httpClient.lastPostHeaders.getFirst("X-AIDefense-Key")).isEqualTo("secret-key");
assertThat(httpClient.lastPostUri).doesNotContain("secret-key");
assertThat(body.get("llm_consensus_runs")).isEqualTo(3);
assertThat(body.get("policy")).isEqualTo("strict");
}
@Test
@ -145,6 +170,7 @@ class SkillScannerServiceTest {
private Object multipartResponse;
private String lastPostUri;
private Object lastPostBody;
private HttpHeaders lastPostHeaders;
private String lastMultipartUri;
private MultiValueMap<String, Object> lastMultipartParts;
private HttpHeaders lastMultipartHeaders;
@ -164,6 +190,15 @@ class SkillScannerServiceTest {
return (T) postResponse;
}
@Override
@SuppressWarnings("unchecked")
public <T> T post(String uri, Object body, HttpHeaders headers, Class<T> responseType) {
this.lastPostUri = uri;
this.lastPostBody = body;
this.lastPostHeaders = headers;
return (T) postResponse;
}
@Override
@SuppressWarnings("unchecked")
public <T> T postMultipart(String uri, MultiValueMap<String, Object> parts, Class<T> responseType) {