Skip to content

Commit a85298f

Browse files
authored
Merge pull request #508 from iflytek/fix/security-review-hardening
fix(security): harden review findings
2 parents a23cbd8 + e501402 commit a85298f

59 files changed

Lines changed: 1567 additions & 106 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.env.release.draft

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,6 @@ SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST=
9393
SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M
9494
SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=noreply@example.com
9595
SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub
96+
97+
# Required for signing anonymous download rate-limit cookies. Use a unique random value per deployment.
98+
SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=replace-with-random-download-secret-32-bytes

.env.release.example

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub
104104
# Security scanner is enabled by default. Set to false to disable scanning.
105105
SKILLHUB_SECURITY_SCANNER_ENABLED=true
106106

107+
# Required for signing anonymous download rate-limit cookies. Use a unique random value per deployment.
108+
# runtime.sh generates and persists one automatically when this placeholder is still present.
109+
SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=replace-with-random-download-secret-32-bytes
110+
107111
# Scanner LLM configuration (optional, for AI-powered scanning features)
108112
SKILL_SCANNER_LLM_API_KEY=
109113
SKILL_SCANNER_LLM_BASE_URL=

.github/workflows/pr-cli.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ on:
77
- 'Makefile'
88
- '.github/workflows/pr-cli.yml'
99

10+
permissions:
11+
contents: read
12+
1013
jobs:
1114
cli:
1215
strategy:
@@ -16,6 +19,8 @@ jobs:
1619
runs-on: ${{ matrix.os }}
1720
steps:
1821
- uses: actions/checkout@v4
22+
with:
23+
persist-credentials: false
1924
- uses: oven-sh/setup-bun@v2
2025
with:
2126
bun-version: 1.3.13

.github/workflows/pr-e2e.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ jobs:
3434
steps:
3535
- name: Check out repository
3636
uses: actions/checkout@v4
37+
with:
38+
persist-credentials: false
3739

3840
- name: Set up pnpm
3941
uses: pnpm/action-setup@v4

.github/workflows/pr-scripts.yml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,33 @@ on:
44
pull_request:
55
paths:
66
- 'scripts/**'
7+
- '.env.release.example'
8+
- '.env.release.draft'
9+
- 'compose.release.yml'
10+
- 'Makefile'
11+
- '.github/workflows/pr-cli.yml'
12+
- '.github/workflows/pr-e2e.yml'
13+
- '.github/workflows/pr-tests.yml'
14+
- '.github/workflows/security.yml'
715
- '.github/workflows/pr-scripts.yml'
816

17+
permissions:
18+
contents: read
19+
920
jobs:
10-
publish-cli-test:
21+
scripts-tests:
22+
name: Script Regression Tests
1123
runs-on: ubuntu-latest
1224
steps:
1325
- uses: actions/checkout@v4
1426
with:
1527
fetch-depth: 0
28+
persist-credentials: false
1629
- uses: actions/setup-node@v4
1730
with:
1831
node-version: '21'
1932
- run: bash scripts/tests/publish-cli-test.sh
33+
- run: bash scripts/tests/runtime-secret-test.sh
34+
- run: bash scripts/tests/validate-release-config-test.sh
35+
- run: bash scripts/tests/dev-web-host-test.sh
36+
- run: bash scripts/tests/workflow-security-test.sh

.github/workflows/pr-tests.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ jobs:
2525
steps:
2626
- name: Check out repository
2727
uses: actions/checkout@v4
28+
with:
29+
persist-credentials: false
2830

2931
- name: Set up pnpm
3032
uses: pnpm/action-setup@v4
@@ -52,6 +54,8 @@ jobs:
5254
steps:
5355
- name: Check out repository
5456
uses: actions/checkout@v4
57+
with:
58+
persist-credentials: false
5559

5660
- name: Set up Java
5761
uses: actions/setup-java@v4
@@ -74,6 +78,8 @@ jobs:
7478
steps:
7579
- name: Check out repository
7680
uses: actions/checkout@v4
81+
with:
82+
persist-credentials: false
7783

7884
- name: Detect docs changes
7985
id: changed

.github/workflows/security.yml

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
name: Security
2+
3+
on:
4+
pull_request:
5+
types:
6+
- opened
7+
- synchronize
8+
- reopened
9+
- ready_for_review
10+
push:
11+
branches:
12+
- main
13+
schedule:
14+
- cron: '23 3 * * 1'
15+
workflow_dispatch:
16+
17+
permissions:
18+
contents: read
19+
20+
jobs:
21+
dependency-review:
22+
name: Dependency Review
23+
if: ${{ github.event_name == 'pull_request' && !github.event.pull_request.draft }}
24+
runs-on: ubuntu-latest
25+
permissions:
26+
contents: read
27+
pull-requests: read
28+
steps:
29+
- name: Check out repository
30+
uses: actions/checkout@v4
31+
with:
32+
persist-credentials: false
33+
34+
- name: Review dependency changes
35+
uses: actions/dependency-review-action@v4
36+
37+
codeql:
38+
name: CodeQL (${{ matrix.language }})
39+
if: ${{ github.event_name != 'pull_request' }}
40+
runs-on: ubuntu-latest
41+
permissions:
42+
contents: read
43+
security-events: write
44+
45+
strategy:
46+
fail-fast: false
47+
matrix:
48+
include:
49+
- language: java-kotlin
50+
build-mode: manual
51+
- language: javascript-typescript
52+
build-mode: none
53+
- language: python
54+
build-mode: none
55+
56+
steps:
57+
- name: Check out repository
58+
uses: actions/checkout@v4
59+
with:
60+
persist-credentials: false
61+
62+
- name: Set up Java
63+
if: matrix.language == 'java-kotlin'
64+
uses: actions/setup-java@v4
65+
with:
66+
distribution: temurin
67+
java-version: 21
68+
cache: maven
69+
70+
- name: Ensure Maven wrapper is executable
71+
if: matrix.language == 'java-kotlin'
72+
run: chmod +x server/mvnw
73+
74+
- name: Initialize CodeQL
75+
uses: github/codeql-action/init@v3
76+
with:
77+
languages: ${{ matrix.language }}
78+
build-mode: ${{ matrix.build-mode }}
79+
80+
- name: Build Java for CodeQL
81+
if: matrix.language == 'java-kotlin'
82+
run: cd server && ./mvnw -q -DskipTests package
83+
84+
- name: Analyze
85+
uses: github/codeql-action/analyze@v3
86+
with:
87+
category: /language:${{ matrix.language }}

Makefile

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ DEV_WEB_PID := $(DEV_DIR)/web.pid
66
DEV_SERVER_LOG := $(DEV_DIR)/server.log
77
DEV_WEB_LOG := $(DEV_DIR)/web.log
88
DEV_WEB_URL := http://localhost:3000
9+
DEV_WEB_HOST ?= 127.0.0.1
910
DEV_API_URL := http://localhost:8080
1011
DEV_SCANNER_URL := http://localhost:8000
1112
STAGING_API_URL := http://localhost:8080
@@ -48,7 +49,7 @@ dev-all: ## 一键启动本地开发环境(依赖 + scanner + 后端 + 前端
4849
echo "Frontend already running with PID $$(cat $(DEV_WEB_PID))"; \
4950
else \
5051
echo "Starting frontend..."; \
51-
$(DEV_PROCESS) start --pid-file $(DEV_WEB_PID) --log-file $(DEV_WEB_LOG) --cwd web -- pnpm exec vite --host 0.0.0.0 --strictPort >/dev/null; \
52+
$(DEV_PROCESS) start --pid-file $(DEV_WEB_PID) --log-file $(DEV_WEB_LOG) --cwd web -- pnpm exec vite --host $(DEV_WEB_HOST) --strictPort >/dev/null; \
5253
fi
5354
@echo "Waiting for backend on $(DEV_API_URL) ..."
5455
@backend_ready=0; \
@@ -126,7 +127,7 @@ dev-all: ## 一键启动本地开发环境(依赖 + scanner + 后端 + 前端
126127
@echo " Frontend: $(DEV_WEB_LOG)"
127128

128129
dev-server: ## 启动后端开发服务器
129-
cd server && /bin/sh -lc '$(DEV_SERVER_PREPARE) && exec $(DEV_SERVER_CMD)'
130+
cd server && /bin/sh -lc '$(DEV_SERVER_PREPARE) && exec env $(DEV_SERVER_SCANNER_ENV) $(DEV_SERVER_CMD)'
130131

131132
dev-server-restart: ## 重启后端开发服务器
132133
@mkdir -p $(DEV_DIR)
@@ -237,7 +238,7 @@ web-install-ci: ## 以 CI 方式安装前端依赖
237238
cd web && CI=true pnpm install --frozen-lockfile
238239

239240
dev-web: ## 启动前端开发服务器
240-
cd web && pnpm run dev
241+
cd web && pnpm exec vite --host $(DEV_WEB_HOST)
241242

242243
build-frontend: web-deps ## 构建前端
243244
cd web && pnpm run build

cli/src/platform/archive.ts

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,30 @@
11
import { mkdir, readdir, readFile, writeFile } from 'node:fs/promises'
22
import { dirname, isAbsolute, join, relative, resolve } from 'node:path'
33
import { zipSync, unzipSync } from 'fflate'
4+
import { MAX_PACKAGE_BYTES } from './download'
5+
6+
const MAX_ZIP_ENTRIES = 500
7+
const MAX_SINGLE_FILE_BYTES = 10 * 1024 * 1024
8+
const EOCD_SIGNATURE = 0x06054b50
9+
const CENTRAL_DIRECTORY_SIGNATURE = 0x02014b50
10+
const ZIP64_MARKER_16 = 0xffff
11+
const ZIP64_MARKER_32 = 0xffffffff
412

513
/**
614
* Extract a zip archive buffer into the target directory.
715
* Pure JS implementation using fflate — no system commands needed.
816
*/
917
export async function extractZip(buffer: ArrayBuffer, targetDir: string): Promise<void> {
1018
await mkdir(targetDir, { recursive: true })
11-
const files = unzipSync(new Uint8Array(buffer))
12-
for (const [name, data] of Object.entries(files)) {
13-
const filePath = safeJoin(targetDir, name)
19+
const archive = new Uint8Array(buffer)
20+
validateZipCentralDirectory(archive)
21+
const files = unzipSync(archive)
22+
const entries = Object.entries(files).map(([name, data]) => ({
23+
name,
24+
data,
25+
filePath: safeJoin(targetDir, name),
26+
}))
27+
for (const { name, data, filePath } of entries) {
1428
if (name.endsWith('/')) {
1529
await mkdir(filePath, { recursive: true })
1630
continue
@@ -20,6 +34,78 @@ export async function extractZip(buffer: ArrayBuffer, targetDir: string): Promis
2034
}
2135
}
2236

37+
function validateZipCentralDirectory(archive: Uint8Array): void {
38+
const view = new DataView(archive.buffer, archive.byteOffset, archive.byteLength)
39+
const eocdOffset = findEndOfCentralDirectory(view)
40+
if (eocdOffset < 0) {
41+
throw new Error('invalid zip central directory')
42+
}
43+
44+
const diskNumber = view.getUint16(eocdOffset + 4, true)
45+
const centralDirectoryDisk = view.getUint16(eocdOffset + 6, true)
46+
const entriesOnDisk = view.getUint16(eocdOffset + 8, true)
47+
const totalEntries = view.getUint16(eocdOffset + 10, true)
48+
const centralDirectorySize = view.getUint32(eocdOffset + 12, true)
49+
const centralDirectoryOffset = view.getUint32(eocdOffset + 16, true)
50+
51+
if (
52+
entriesOnDisk === ZIP64_MARKER_16 ||
53+
totalEntries === ZIP64_MARKER_16 ||
54+
centralDirectorySize === ZIP64_MARKER_32 ||
55+
centralDirectoryOffset === ZIP64_MARKER_32
56+
) {
57+
throw new Error('zip64 archives are not supported')
58+
}
59+
if (diskNumber !== 0 || centralDirectoryDisk !== 0 || entriesOnDisk !== totalEntries) {
60+
throw new Error('multi-disk zip archives are not supported')
61+
}
62+
if (totalEntries > MAX_ZIP_ENTRIES) {
63+
throw new Error('zip entry count exceeds limit')
64+
}
65+
if (centralDirectoryOffset + centralDirectorySize > archive.byteLength) {
66+
throw new Error('invalid zip central directory')
67+
}
68+
69+
let offset = centralDirectoryOffset
70+
let totalUncompressedSize = 0
71+
const decoder = new TextDecoder()
72+
for (let i = 0; i < totalEntries; i++) {
73+
if (offset + 46 > archive.byteLength || view.getUint32(offset, true) !== CENTRAL_DIRECTORY_SIGNATURE) {
74+
throw new Error('invalid zip central directory')
75+
}
76+
const uncompressedSize = view.getUint32(offset + 24, true)
77+
const nameLength = view.getUint16(offset + 28, true)
78+
const extraLength = view.getUint16(offset + 30, true)
79+
const commentLength = view.getUint16(offset + 32, true)
80+
const nameStart = offset + 46
81+
const nameEnd = nameStart + nameLength
82+
const nextOffset = nameEnd + extraLength + commentLength
83+
if (nameEnd > archive.byteLength || nextOffset > archive.byteLength) {
84+
throw new Error('invalid zip central directory')
85+
}
86+
87+
const entryName = decoder.decode(archive.subarray(nameStart, nameEnd))
88+
if (!entryName.endsWith('/') && uncompressedSize > MAX_SINGLE_FILE_BYTES) {
89+
throw new Error('zip entry size exceeds limit')
90+
}
91+
totalUncompressedSize += uncompressedSize
92+
if (totalUncompressedSize > MAX_PACKAGE_BYTES) {
93+
throw new Error('zip total uncompressed size exceeds limit')
94+
}
95+
offset = nextOffset
96+
}
97+
}
98+
99+
function findEndOfCentralDirectory(view: DataView): number {
100+
const minOffset = Math.max(0, view.byteLength - 0xffff - 22)
101+
for (let offset = view.byteLength - 22; offset >= minOffset; offset--) {
102+
if (view.getUint32(offset, true) === EOCD_SIGNATURE) {
103+
return offset
104+
}
105+
}
106+
return -1
107+
}
108+
23109
/**
24110
* Create a zip archive from a directory.
25111
* Returns the archive as a Blob.

0 commit comments

Comments
 (0)