diff --git a/.env.release.example b/.env.release.example index 40e51ac1..2d30c7bc 100644 --- a/.env.release.example +++ b/.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) diff --git a/.github/workflows/pr-scripts.yml b/.github/workflows/pr-scripts.yml index 2f8cc80d..d15ee4bd 100644 --- a/.github/workflows/pr-scripts.yml +++ b/.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 diff --git a/cli/src/platform/archive.ts b/cli/src/platform/archive.ts index c1306711..f549185d 100644 --- a/cli/src/platform/archive.ts +++ b/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 diff --git a/cli/test/unit/platform/archive.test.ts b/cli/test/unit/platform/archive.test.ts index 98972134..b175e279 100644 --- a/cli/test/unit/platform/archive.test.ts +++ b/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') }) diff --git a/scripts/runtime.sh b/scripts/runtime.sh index f5b89550..f46e7c3e 100755 --- a/scripts/runtime.sh +++ b/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() { diff --git a/scripts/tests/runtime-secret-test.sh b/scripts/tests/runtime-secret-test.sh new file mode 100755 index 00000000..34dd4fab --- /dev/null +++ b/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" diff --git a/scripts/tests/validate-release-config-test.sh b/scripts/tests/validate-release-config-test.sh index 3a52d7a3..fd76ddfc 100755 --- a/scripts/tests/validate-release-config-test.sh +++ b/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" diff --git a/scripts/tests/workflow-security-test.sh b/scripts/tests/workflow-security-test.sh index 8219af51..94cb2556 100755 --- a/scripts/tests/workflow-security-test.sh +++ b/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" \ diff --git a/scripts/validate-release-config.sh b/scripts/validate-release-config.sh index 758b168f..03e42bd8 100755 --- a/scripts/validate-release-config.sh +++ b/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