fix(security): address review blockers

Signed-off-by: dongmucat <1127093059@qq.com>

.env.release.example

@@ -105,6 +105,7 @@ SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub
 SKILLHUB_SECURITY_SCANNER_ENABLED=true
 
 # Required for signing anonymous download rate-limit cookies. Use a unique random value per deployment.
+# runtime.sh generates and persists one automatically when this placeholder is still present.
 SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=replace-with-random-download-secret-32-bytes
 
 # Scanner LLM configuration (optional, for AI-powered scanning features)

.github/workflows/pr-scripts.yml

@@ -8,6 +8,9 @@ on:
       - '.github/workflows/security.yml'
       - '.github/workflows/pr-scripts.yml'
 
+permissions:
+  contents: read
+
 jobs:
   scripts-tests:
     name: Script Regression Tests
@@ -16,10 +19,12 @@ jobs:
       - uses: actions/checkout@v4
         with:
           fetch-depth: 0
+          persist-credentials: false
       - uses: actions/setup-node@v4
         with:
           node-version: '21'
       - run: bash scripts/tests/publish-cli-test.sh
+      - run: bash scripts/tests/runtime-secret-test.sh
       - run: bash scripts/tests/validate-release-config-test.sh
       - run: bash scripts/tests/dev-web-host-test.sh
       - run: bash scripts/tests/workflow-security-test.sh

cli/src/platform/archive.ts

@@ -19,8 +19,12 @@ export async function extractZip(buffer: ArrayBuffer, targetDir: string): Promis
   const archive = new Uint8Array(buffer)
   validateZipCentralDirectory(archive)
   const files = unzipSync(archive)
-  for (const [name, data] of Object.entries(files)) {
-    const filePath = safeJoin(targetDir, name)
+  const entries = Object.entries(files).map(([name, data]) => ({
+    name,
+    data,
+    filePath: safeJoin(targetDir, name),
+  }))
+  for (const { name, data, filePath } of entries) {
     if (name.endsWith('/')) {
       await mkdir(filePath, { recursive: true })
       continue

cli/test/unit/platform/archive.test.ts

@@ -32,6 +32,17 @@ describe('archive helpers', () => {
     await expect(extractZip(unsafe.buffer as ArrayBuffer, target)).rejects.toThrow('unsafe zip entry path')
   })
 
+  test('rejects unsafe zip before writing earlier safe entries', async () => {
+    const target = await mkdtemp(join(tmpdir(), 'skillhub-archive-partial-'))
+    const unsafe = zipSync({
+      'SKILL.md': new TextEncoder().encode('# Partial'),
+      '../escape.txt': new TextEncoder().encode('bad'),
+    })
+
+    await expect(extractZip(unsafe.buffer as ArrayBuffer, target)).rejects.toThrow('unsafe zip entry path')
+    await expect(readFile(join(target, 'SKILL.md'), 'utf-8')).rejects.toThrow()
+  })
+
   test('rejects zip entries with absolute paths', async () => {
     const target = await mkdtemp(join(tmpdir(), 'skillhub-archive-abs-'))
     const unsafe = zipSync({ '/etc/passwd': new TextEncoder().encode('bad') })

scripts/runtime.sh

@@ -180,6 +180,42 @@ get_env_value() {
   fi
 }
 
+generate_secret() {
+  if command -v openssl >/dev/null 2>&1; then
+    openssl rand -hex 32
+    return 0
+  fi
+
+  if [ -r /dev/urandom ] && command -v od >/dev/null 2>&1; then
+    dd if=/dev/urandom bs=32 count=1 2>/dev/null | od -An -tx1 | tr -d ' \n'
+    return 0
+  fi
+
+  echo "Unable to generate SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET. Install openssl or configure it manually." >&2
+  exit 1
+}
+
+is_placeholder_secret() {
+  case "$1" in
+    ""|change-me-in-production|replace-me|replace-with-random-download-secret-32-bytes|TODO*|todo*|replace*)
+      return 0
+      ;;
+    *)
+      return 1
+      ;;
+  esac
+}
+
+ensure_anonymous_download_secret() {
+  secret="$(get_env_value "SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET" "")"
+  if ! is_placeholder_secret "$secret" && [ "${#secret}" -ge 32 ]; then
+    return 0
+  fi
+
+  set_env_value "SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET" "$(generate_secret)"
+  echo "Generated SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET in $ENV_FILE"
+}
+
 wait_for_postgres_ready() {
   postgres_user="$1"
   postgres_db="$2"
@@ -280,6 +316,8 @@ prepare_runtime_files() {
   if [ -n "$SKILLHUB_PUBLIC_BASE_URL_VALUE" ]; then
     set_env_value "SKILLHUB_PUBLIC_BASE_URL" "$SKILLHUB_PUBLIC_BASE_URL_VALUE"
   fi
+
+  ensure_anonymous_download_secret
 }
 
 run_compose() {

scripts/tests/runtime-secret-test.sh

@@ -0,0 +1,99 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+SCRIPT="$REPO_ROOT/scripts/runtime.sh"
+
+TMP_DIRS=()
+cleanup() {
+  local d
+  for d in "${TMP_DIRS[@]+"${TMP_DIRS[@]}"}"; do
+    rm -rf "$d"
+  done
+}
+trap cleanup EXIT
+
+new_tmp() {
+  local d
+  d="$(mktemp -d)"
+  TMP_DIRS+=("$d")
+  echo "$d"
+}
+
+fail() {
+  echo "FAIL: $*" >&2
+  exit 1
+}
+
+install_fake_tools() {
+  local bin_dir="$1"
+  mkdir -p "$bin_dir"
+  cat >"$bin_dir/docker" <<'EOF'
+#!/usr/bin/env bash
+set -euo pipefail
+printf '%s\n' "$*" >> "${DOCKER_LOG:?DOCKER_LOG is required}"
+if [[ "${1:-}" == "compose" && "${2:-}" == "version" ]]; then
+  exit 0
+fi
+exit 0
+EOF
+  chmod +x "$bin_dir/docker"
+
+  cat >"$bin_dir/openssl" <<'EOF'
+#!/usr/bin/env bash
+set -euo pipefail
+if [[ "${1:-}" == "rand" && "${2:-}" == "-hex" && "${3:-}" == "32" ]]; then
+  printf '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef\n'
+  exit 0
+fi
+echo "unsupported openssl args: $*" >&2
+exit 1
+EOF
+  chmod +x "$bin_dir/openssl"
+}
+
+run_runtime() {
+  local home="$1"
+  local bin_dir="$2"
+  local stdout="$3"
+  DOCKER_LOG="$home/docker.log" \
+    SKILLHUB_HOME="$home" \
+    SKILLHUB_RAW_BASE="file://$REPO_ROOT" \
+    PATH="$bin_dir:$PATH" \
+    sh "$SCRIPT" up --version sha-test --public-url http://localhost >"$stdout"
+}
+
+tmp="$(new_tmp)"
+bin_dir="$tmp/bin"
+install_fake_tools "$bin_dir"
+
+home_generated="$tmp/generated"
+stdout_generated="$tmp/generated.out"
+mkdir -p "$home_generated"
+run_runtime "$home_generated" "$bin_dir" "$stdout_generated"
+
+generated_secret="$(grep '^SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=' "$home_generated/.env.release" | cut -d= -f2-)"
+[[ "$generated_secret" == "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" ]] \
+  || fail "runtime should generate a persisted anonymous download secret"
+grep -Fq "Generated SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET" "$stdout_generated" \
+  || fail "runtime should explain that it generated the secret"
+if grep -Fq "$generated_secret" "$stdout_generated"; then
+  fail "runtime must not print the generated secret value"
+fi
+
+home_preserved="$tmp/preserved"
+stdout_preserved="$tmp/preserved.out"
+mkdir -p "$home_preserved"
+cat >"$home_preserved/.env.release" <<'EOF'
+SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=already-valid-runtime-secret-32-bytes
+EOF
+run_runtime "$home_preserved" "$bin_dir" "$stdout_preserved"
+
+preserved_secret="$(grep '^SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=' "$home_preserved/.env.release" | cut -d= -f2-)"
+[[ "$preserved_secret" == "already-valid-runtime-secret-32-bytes" ]] \
+  || fail "runtime must preserve an existing valid anonymous download secret"
+if grep -Fq "Generated SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET" "$stdout_preserved"; then
+  fail "runtime must not regenerate an existing valid secret"
+fi
+
+echo "runtime-secret-test passed"

scripts/tests/validate-release-config-test.sh

@@ -80,4 +80,17 @@ short_env="$tmp/short.env"
 write_env "$short_env" "too-short"
 expect_fail "$short_env" "SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET must be at least 32 characters"
 
+draft_env="$tmp/draft.env"
+while IFS= read -r line || [[ -n "$line" ]]; do
+  case "$line" in
+    SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=*)
+      printf '%s\n' "SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=release-download-secret-32-bytes-minimum"
+      ;;
+    *)
+      printf '%s\n' "$line"
+      ;;
+  esac
+done <"$REPO_ROOT/.env.release.draft" >"$draft_env"
+expect_fail "$draft_env" "POSTGRES_PASSWORD"
+
 echo "validate-release-config-test passed"

scripts/tests/workflow-security-test.sh

@@ -12,6 +12,13 @@ fail() {
 
 [[ -f "$SECURITY_WORKFLOW" ]] || fail ".github/workflows/security.yml is required"
 
+grep -Eq '^permissions:[[:space:]]*$' "$PR_SCRIPTS_WORKFLOW" \
+  || fail "pr-scripts must declare top-level permissions"
+grep -Eq '^[[:space:]]+contents:[[:space:]]+read[[:space:]]*$' "$PR_SCRIPTS_WORKFLOW" \
+  || fail "pr-scripts GITHUB_TOKEN permissions must be read-only"
+grep -Fq 'persist-credentials: false' "$PR_SCRIPTS_WORKFLOW" \
+  || fail "pr-scripts checkout must not persist credentials"
+
 grep -Fq 'actions/dependency-review-action' "$SECURITY_WORKFLOW" \
   || fail "security workflow must run dependency review"
 grep -Fq 'github/codeql-action/init' "$SECURITY_WORKFLOW" \
@@ -25,6 +32,8 @@ grep -Fq '.github/workflows/security.yml' "$PR_SCRIPTS_WORKFLOW" \
   || fail "pr-scripts must run when security workflow changes"
 grep -Fq 'bash scripts/tests/validate-release-config-test.sh' "$PR_SCRIPTS_WORKFLOW" \
   || fail "pr-scripts must run validate-release-config-test"
+grep -Fq 'bash scripts/tests/runtime-secret-test.sh' "$PR_SCRIPTS_WORKFLOW" \
+  || fail "pr-scripts must run runtime-secret-test"
 grep -Fq 'bash scripts/tests/dev-web-host-test.sh' "$PR_SCRIPTS_WORKFLOW" \
   || fail "pr-scripts must run dev-web-host-test"
 grep -Fq 'bash scripts/tests/workflow-security-test.sh' "$PR_SCRIPTS_WORKFLOW" \

scripts/validate-release-config.sh

@@ -52,6 +52,23 @@ reject_values() {
   done
 }
 
+reject_patterns() {
+  var_name="$1"
+  shift
+  eval "var_value=\${$var_name:-}"
+  if [ -z "$var_value" ]; then
+    return 0
+  fi
+  for pattern in "$@"; do
+    case "$var_value" in
+      $pattern)
+        error "$var_name still uses placeholder/default pattern: $var_value"
+        return 0
+        ;;
+    esac
+  done
+}
+
 validate_url() {
   var_name="$1"
   eval "var_value=\${$var_name:-}"
@@ -115,15 +132,22 @@ validate_no_trailing_slash SKILLHUB_PUBLIC_BASE_URL
 
 require_non_empty SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET
 reject_values SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET "change-me-in-production" "replace-me" "replace-with-random-download-secret-32-bytes"
+reject_patterns SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET "TODO_*" "todo_*" "replace*"
 validate_min_length SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET 32
 
 reject_values POSTGRES_PASSWORD "change-this-postgres-password" "skillhub_demo" "skillhub_dev"
+reject_patterns POSTGRES_PASSWORD "TODO_*" "todo_*"
 reject_values BOOTSTRAP_ADMIN_PASSWORD "replace-this-admin-password" "ChangeMe!2026" "Admin@2026"
+reject_patterns BOOTSTRAP_ADMIN_PASSWORD "TODO_*" "todo_*" "replace*"
 if [ "${BOOTSTRAP_ADMIN_ENABLED:-false}" = "true" ]; then
   require_non_empty BOOTSTRAP_ADMIN_PASSWORD
 fi
 reject_values SKILLHUB_STORAGE_S3_ACCESS_KEY "replace-me"
 reject_values SKILLHUB_STORAGE_S3_SECRET_KEY "replace-me"
+reject_patterns SKILLHUB_STORAGE_S3_ACCESS_KEY "TODO_*" "todo_*" "replace*"
+reject_patterns SKILLHUB_STORAGE_S3_SECRET_KEY "TODO_*" "todo_*" "replace*"
+reject_patterns SPRING_MAIL_USERNAME "TODO_*" "todo_*" "replace*"
+reject_patterns SPRING_MAIL_PASSWORD "TODO_*" "todo_*" "replace*"
 
 validate_boolean SESSION_COOKIE_SECURE
 validate_boolean BOOTSTRAP_ADMIN_ENABLED