mirror of
https://github.com/iflytek/skillhub.git
synced 2026-08-27 11:14:59 +00:00
Merge 36967794d1 into 26f49e6819
This commit is contained in:
commit
0622e0bfaa
9 changed files with 93 additions and 32 deletions
|
|
@ -4,6 +4,7 @@ import com.iflytek.skillhub.auth.token.ApiTokenAuthenticationFilter;
|
|||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.core.annotation.Order;
|
||||
import org.springframework.http.HttpMethod;
|
||||
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
|
||||
import org.springframework.security.config.http.SessionCreationPolicy;
|
||||
import org.springframework.security.web.SecurityFilterChain;
|
||||
|
|
@ -24,12 +25,11 @@ public class ClawHubRegistrySecurityConfig {
|
|||
http
|
||||
.securityMatcher(
|
||||
new OrRequestMatcher(
|
||||
new AntPathRequestMatcher("/api/v1/labels"),
|
||||
new AntPathRequestMatcher("/api/web/labels")
|
||||
new AntPathRequestMatcher("/api/v1/labels", HttpMethod.GET.name()),
|
||||
new AntPathRequestMatcher("/api/web/labels", HttpMethod.GET.name())
|
||||
)
|
||||
)
|
||||
.authorizeHttpRequests(auth -> auth.anyRequest().permitAll())
|
||||
.csrf(csrf -> csrf.disable())
|
||||
.requestCache(cache -> cache.disable())
|
||||
.securityContext(context -> context.disable())
|
||||
.sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import org.springframework.test.web.servlet.MockMvc;
|
|||
|
||||
import static org.mockito.Mockito.when;
|
||||
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.status;
|
||||
|
||||
|
|
@ -60,4 +61,10 @@ class LabelControllerTest {
|
|||
.andExpect(jsonPath("$.data[0].slug").value("code-generation"))
|
||||
.andExpect(jsonPath("$.data[0].displayName").value("Code Generation"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void labelMutationShouldNotUseThePublicGetOnlySecurityChain() throws Exception {
|
||||
mockMvc.perform(post("/api/v1/labels"))
|
||||
.andExpect(status().isUnauthorized());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
package com.iflytek.skillhub.domain.skill.metadata;
|
||||
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
|
||||
import org.yaml.snakeyaml.LoaderOptions;
|
||||
import org.yaml.snakeyaml.Yaml;
|
||||
import org.yaml.snakeyaml.constructor.SafeConstructor;
|
||||
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.Map;
|
||||
|
|
@ -13,6 +15,9 @@ import java.util.Map;
|
|||
public class SkillMetadataParser {
|
||||
|
||||
private static final String FRONTMATTER_DELIMITER = "---";
|
||||
private static final int MAX_YAML_ALIASES = 20;
|
||||
private static final int MAX_YAML_NESTING_DEPTH = 20;
|
||||
private static final int MAX_YAML_CODE_POINTS = 200_000;
|
||||
|
||||
public SkillMetadata parse(String content) {
|
||||
if (content == null || content.isBlank()) {
|
||||
|
|
@ -59,7 +64,12 @@ public class SkillMetadataParser {
|
|||
|
||||
private Map<String, Object> parseFrontmatter(String yamlContent) {
|
||||
try {
|
||||
Yaml yaml = new Yaml();
|
||||
LoaderOptions loaderOptions = new LoaderOptions();
|
||||
loaderOptions.setAllowDuplicateKeys(false);
|
||||
loaderOptions.setMaxAliasesForCollections(MAX_YAML_ALIASES);
|
||||
loaderOptions.setNestingDepthLimit(MAX_YAML_NESTING_DEPTH);
|
||||
loaderOptions.setCodePointLimit(MAX_YAML_CODE_POINTS);
|
||||
Yaml yaml = new Yaml(new SafeConstructor(loaderOptions));
|
||||
Object parsed = yaml.load(yamlContent);
|
||||
if (!(parsed instanceof Map)) {
|
||||
throw new DomainBadRequestException("error.skill.metadata.yaml.notMap");
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import java.nio.charset.StandardCharsets;
|
|||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
|
|
@ -16,9 +17,9 @@ import java.util.regex.Pattern;
|
|||
@Component
|
||||
public class BasicPrePublishValidator implements PrePublishValidator {
|
||||
|
||||
private static final Pattern PLACEHOLDER_VALUE = Pattern.compile(
|
||||
"(?i).*(your|example|sample|placeholder|changeme|replace|dummy|mock|test|fake|todo|xxx|redacted).*"
|
||||
);
|
||||
private static final Set<String> PLACEHOLDER_MARKERS = Set.of(
|
||||
"your", "example", "sample", "placeholder", "changeme", "replace", "dummy",
|
||||
"mock", "test", "fake", "todo", "xxx", "redacted");
|
||||
private static final List<SecretRule> SECRET_RULES = List.of(
|
||||
new SecretRule(Pattern.compile("(AKIA[0-9A-Z]{16})"), 1, "cloud access key"),
|
||||
new SecretRule(Pattern.compile("(ghp_[A-Za-z0-9]{20,})"), 1, "GitHub token"),
|
||||
|
|
@ -83,7 +84,8 @@ public class BasicPrePublishValidator implements PrePublishValidator {
|
|||
if (value == null || value.isBlank()) {
|
||||
return false;
|
||||
}
|
||||
return PLACEHOLDER_VALUE.matcher(value).matches()
|
||||
String normalizedValue = value.toLowerCase(Locale.ROOT);
|
||||
return PLACEHOLDER_MARKERS.stream().anyMatch(normalizedValue::contains)
|
||||
|| value.chars().allMatch(ch -> ch == 'x' || ch == 'X' || ch == '*' || ch == '-');
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -199,6 +199,22 @@ class SkillMetadataParserTest {
|
|||
assertEquals("1.0.0", metadata.version());
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDoesNotInstantiateUnsafeGlobalYamlTags() {
|
||||
String content = """
|
||||
---
|
||||
name: safe-skill
|
||||
description: Reject unsafe YAML object construction
|
||||
payload: !!java.util.Date []
|
||||
---
|
||||
Body
|
||||
""";
|
||||
|
||||
SkillMetadata metadata = parser.parse(content);
|
||||
|
||||
assertEquals("!!java.util.Date []", metadata.frontmatter().get("payload"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testThrowsWhenNoClosingDelimiter() {
|
||||
String content = """
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { expect, type Page, type TestInfo } from '@playwright/test'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { csrfHeaders } from './csrf'
|
||||
|
||||
const password = 'Passw0rd!123'
|
||||
|
|
@ -38,7 +39,7 @@ function usernameForWorker(testInfo?: TestInfo): string {
|
|||
|
||||
function uniqueUsernameForWorker(testInfo?: TestInfo): string {
|
||||
const worker = testInfo?.parallelIndex ?? 0
|
||||
const suffix = `${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`
|
||||
const suffix = `${Date.now().toString(36)}${randomUUID().slice(0, 8)}`
|
||||
return `e2e_w${worker}_${suffix}`
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
import { expect, test } from '@playwright/test'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { setEnglishLocale } from './helpers/auth-fixtures'
|
||||
|
||||
function buildUniqueUser() {
|
||||
const suffix = `${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`
|
||||
const suffix = `${Date.now().toString(36)}${randomUUID().slice(0, 8)}`
|
||||
return {
|
||||
username: `e2e_reg_${suffix}`,
|
||||
email: `e2e_reg_${suffix}@example.test`,
|
||||
|
|
|
|||
|
|
@ -1,9 +1,5 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// CliAuthPage has internal helpers isValidRedirectUri and decodeLabel which are
|
||||
// not exported. We test the component render paths and validate the redirect
|
||||
// URI logic via the rendered error states.
|
||||
|
||||
vi.mock('@tanstack/react-router', () => ({
|
||||
useNavigate: () => vi.fn(),
|
||||
}))
|
||||
|
|
@ -36,7 +32,32 @@ vi.mock('@/app/router', () => ({
|
|||
ORIGINAL_URL_SEARCH: '',
|
||||
}))
|
||||
|
||||
import { CliAuthPage, resolveCliRegistryUrl } from './cli-auth'
|
||||
import { CliAuthPage, resolveCliRegistryUrl, resolveLoopbackRedirectUri } from './cli-auth'
|
||||
|
||||
describe('resolveLoopbackRedirectUri', () => {
|
||||
it.each([
|
||||
'http://localhost:4312/callback?source=cli',
|
||||
'http://127.0.0.1:4312/callback',
|
||||
'http://[::1]:4312/callback',
|
||||
])('accepts an HTTP loopback callback: %s', (uri) => {
|
||||
expect(resolveLoopbackRedirectUri(uri)?.href).toBe(uri)
|
||||
})
|
||||
|
||||
it.each([
|
||||
'https://localhost:4312/callback',
|
||||
'http://localhost.example.com/callback',
|
||||
'http://example.com/callback',
|
||||
'http://user:password@localhost:4312/callback',
|
||||
'javascript:alert(1)',
|
||||
'not-a-url',
|
||||
])('rejects a non-loopback or unsafe callback: %s', (uri) => {
|
||||
expect(resolveLoopbackRedirectUri(uri)).toBeNull()
|
||||
})
|
||||
|
||||
it('removes an attacker-provided fragment before adding CLI credentials', () => {
|
||||
expect(resolveLoopbackRedirectUri('http://localhost:4312/callback#attacker')?.hash).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveCliRegistryUrl', () => {
|
||||
it('uses the configured public base URL for the CLI registry', () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { useState, useEffect } from 'react'
|
||||
import { useEffect, useMemo, useState } from 'react'
|
||||
import { useNavigate } from '@tanstack/react-router'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
import { Card } from '@/shared/ui/card'
|
||||
|
|
@ -12,14 +12,19 @@ import { resolvePublicRegistryUrl } from '@/shared/lib/registry-url'
|
|||
// Parse the original URL params captured before TanStack Router rewrites
|
||||
const ORIGINAL_PARAMS = new URLSearchParams(ORIGINAL_URL_SEARCH)
|
||||
|
||||
function isValidRedirectUri(uri: string): boolean {
|
||||
const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]', '::1'])
|
||||
|
||||
export function resolveLoopbackRedirectUri(uri: string): URL | null {
|
||||
try {
|
||||
const url = new URL(uri)
|
||||
// Only allow localhost/127.0.0.1/::1 on HTTP
|
||||
const validHosts = ['localhost', '127.0.0.1', '[::1]', '::1']
|
||||
return url.protocol === 'http:' && validHosts.includes(url.hostname.toLowerCase())
|
||||
const isLoopbackHttp = url.protocol === 'http:' && LOOPBACK_HOSTS.has(url.hostname.toLowerCase())
|
||||
if (!isLoopbackHttp || url.username || url.password) {
|
||||
return null
|
||||
}
|
||||
url.hash = ''
|
||||
return url
|
||||
} catch {
|
||||
return false
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -55,13 +60,10 @@ export function CliAuthPage() {
|
|||
const labelB64 = ORIGINAL_PARAMS.get('label_b64')?.trim() || undefined
|
||||
const labelPlain = ORIGINAL_PARAMS.get('label')?.trim() || undefined
|
||||
const label = decodeLabel(labelB64, labelPlain)
|
||||
|
||||
// Debug: log search params and raw URL
|
||||
console.log('CLI Auth - Original search (from router.tsx):', ORIGINAL_URL_SEARCH)
|
||||
console.log('CLI Auth - Current URL:', typeof window !== 'undefined' ? window.location.href : 'SSR')
|
||||
console.log('CLI Auth - redirectUri:', redirectUri)
|
||||
console.log('CLI Auth - state:', state)
|
||||
console.log('CLI Auth - label:', label)
|
||||
const redirectTarget = useMemo(
|
||||
() => redirectUri ? resolveLoopbackRedirectUri(redirectUri) : null,
|
||||
[redirectUri],
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
// Check authentication status
|
||||
|
|
@ -89,7 +91,7 @@ export function CliAuthPage() {
|
|||
}
|
||||
|
||||
// Validate redirect_uri
|
||||
if (!redirectUri || !isValidRedirectUri(redirectUri)) {
|
||||
if (!redirectTarget) {
|
||||
setStatus('error')
|
||||
setErrorMessage(t('cliAuth.invalidRedirectUri'))
|
||||
return
|
||||
|
|
@ -125,16 +127,17 @@ export function CliAuthPage() {
|
|||
hashParams.set('registry', registryUrl)
|
||||
hashParams.set('state', state)
|
||||
|
||||
const redirectUrl = `${redirectUri}#${hashParams.toString()}`
|
||||
const redirectUrl = new URL(redirectTarget.href)
|
||||
redirectUrl.hash = hashParams.toString()
|
||||
|
||||
// Redirect to CLI's loopback server
|
||||
window.location.assign(redirectUrl)
|
||||
window.location.assign(redirectUrl.href)
|
||||
})
|
||||
.catch((error) => {
|
||||
setStatus('error')
|
||||
setErrorMessage(error instanceof Error ? error.message : t('cliAuth.tokenCreationFailed'))
|
||||
})
|
||||
}, [user, redirectUri, state, label, t])
|
||||
}, [user, redirectUri, redirectTarget, state, label, t])
|
||||
|
||||
if (status === 'validating') {
|
||||
return (
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue