diff --git a/.github/workflows/security-scan-codex.yml b/.github/workflows/security-scan-codex.yml index 1193f334..fa00d4e5 100644 --- a/.github/workflows/security-scan-codex.yml +++ b/.github/workflows/security-scan-codex.yml @@ -42,8 +42,6 @@ jobs: shard: [0, 1, 2, 3] env: CONVEX_URL: ${{ vars.CONVEX_URL || vars.VITE_CONVEX_URL || 'https://wry-manatee-359.convex.cloud' }} - SECURITY_SCAN_WORKER_TOKEN: ${{ secrets.SECURITY_SCAN_WORKER_TOKEN }} - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} CODEX_SECURITY_SCAN_LIMIT: ${{ inputs.limit || inputs['batch-limit'] || '4' }} CODEX_SECURITY_SCAN_MAX_JOBS: ${{ inputs['max-jobs'] || '' }} CODEX_SECURITY_SCAN_MAX_RUNTIME_MINUTES: ${{ inputs['max-runtime-minutes'] || '40' }} @@ -61,18 +59,6 @@ jobs: with: python-version: "3.12" - - name: Check configuration - run: | - set -euo pipefail - if [[ -z "$SECURITY_SCAN_WORKER_TOKEN" ]]; then - echo "::error::SECURITY_SCAN_WORKER_TOKEN is required" - exit 1 - fi - if [[ -z "$OPENAI_API_KEY" ]]; then - echo "::error::OPENAI_API_KEY is required" - exit 1 - fi - - name: Install Codex CLI run: | set -euo pipefail @@ -92,9 +78,14 @@ jobs: skillspector --help >/dev/null - name: Authenticate Codex CLI + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} run: printf '%s' "$OPENAI_API_KEY" | codex login --with-api-key - name: Run Codex security worker + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + SECURITY_SCAN_WORKER_TOKEN: ${{ secrets.SECURITY_SCAN_WORKER_TOKEN }} run: | bun scripts/security/run-codex-scan-worker.ts \ --batch-limit "$CODEX_SECURITY_SCAN_LIMIT" \ @@ -102,8 +93,25 @@ jobs: --max-runtime-minutes "$CODEX_SECURITY_SCAN_MAX_RUNTIME_MINUTES" \ --lease-minutes "$CODEX_SECURITY_SCAN_LEASE_MINUTES" - - name: Upload Codex security diagnostics + - name: Prepare Codex security diagnostics scan if: ${{ !cancelled() }} + run: mkdir -p "$CODEX_SECURITY_SCAN_DIAGNOSTICS_DIR" + + - name: Scan Codex security diagnostics for verified secrets + id: diagnostics_secret_scan + if: ${{ !cancelled() }} + run: | + docker run --rm \ + -v "$PWD/$CODEX_SECURITY_SCAN_DIAGNOSTICS_DIR:/scan:ro" \ + ghcr.io/trufflesecurity/trufflehog:3.95.5@sha256:56c25710275c4b8d74c4f1346a5e7c606fa7ff4afe996f680b288d0fae3fcd9c \ + filesystem /scan \ + --only-verified \ + --fail \ + --no-update \ + --github-actions + + - name: Upload Codex security diagnostics + if: ${{ !cancelled() && steps.diagnostics_secret_scan.outcome == 'success' }} uses: actions/upload-artifact@v7 with: name: codex-security-scan-diagnostics-${{ github.run_id }}-${{ matrix.shard }} diff --git a/.github/workflows/skill-card-worker.yml b/.github/workflows/skill-card-worker.yml index 94163999..ad294ef1 100644 --- a/.github/workflows/skill-card-worker.yml +++ b/.github/workflows/skill-card-worker.yml @@ -39,8 +39,6 @@ jobs: shard: [0, 1, 2, 3] env: CONVEX_URL: ${{ vars.CONVEX_URL || vars.VITE_CONVEX_URL || 'https://wry-manatee-359.convex.cloud' }} - # Shared Convex worker credential used by security and Skill Card workers. - SECURITY_SCAN_WORKER_TOKEN: ${{ secrets.SECURITY_SCAN_WORKER_TOKEN }} SKILL_CARD_WORKER_LIMIT: ${{ github.event.inputs['batch-limit'] || '4' }} SKILL_CARD_WORKER_MAX_JOBS: ${{ github.event.inputs['max-jobs'] || '' }} SKILL_CARD_WORKER_MAX_RUNTIME_MINUTES: ${{ github.event.inputs['max-runtime-minutes'] || '40' }} @@ -59,20 +57,6 @@ jobs: - uses: ./.github/actions/setup-bun - - name: Check configuration - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - run: | - set -euo pipefail - if [[ -z "$SECURITY_SCAN_WORKER_TOKEN" ]]; then - echo "::error::SECURITY_SCAN_WORKER_TOKEN is required" - exit 1 - fi - if [[ -z "$OPENAI_API_KEY" ]]; then - echo "::error::OPENAI_API_KEY is required" - exit 1 - fi - - name: Install Codex CLI and renderer dependencies run: | set -euo pipefail @@ -88,6 +72,9 @@ jobs: run: printf '%s' "$OPENAI_API_KEY" | codex login --with-api-key - name: Run Skill Card worker + env: + # Shared Convex worker credential used by security and Skill Card workers. + SECURITY_SCAN_WORKER_TOKEN: ${{ secrets.SECURITY_SCAN_WORKER_TOKEN }} run: | args=( --batch-limit "$SKILL_CARD_WORKER_LIMIT" diff --git a/bun.lock b/bun.lock index e7168146..e98e94d1 100644 --- a/bun.lock +++ b/bun.lock @@ -43,6 +43,7 @@ "mermaid": "^11.15.0", "mime": "4.1.0", "monaco-editor": "0.55.1", + "pino": "10.3.1", "react": "19.2.7", "react-dom": "19.2.7", "react-markdown": "10.1.0", @@ -557,6 +558,8 @@ "@panva/hkdf": ["@panva/hkdf@1.2.1", "", {}, "sha512-6oclG6Y3PiDFcoyk8srjLfVKyMfVCKJ27JwNPViuXziFpmdz+MZnZN/aKY0JGXgYuO/VghU0jcOAZgWXZ1Dmrw=="], + "@pinojs/redact": ["@pinojs/redact@0.4.0", "", {}, "sha512-k2ENnmBugE/rzQfEcdWHcCY+/FM3VLzH9cYEsbdsoqrvzAKRhUZeRNhAZvB8OitQJ1TBed3yqWtdjzS6wJKBwg=="], + "@playwright/test": ["@playwright/test@1.61.0", "", { "dependencies": { "playwright": "1.61.0" }, "bin": { "playwright": "cli.js" } }, "sha512-cKA5B6lpFEMyMGjxF54QihfYpB4FkEGH+qZhtArDEG+wezQAJY8Pq6C7T1SjWz+FFzt3TbyoXBQYk/0292TdJA=="], "@radix-ui/number": ["@radix-ui/number@1.1.2", "", {}, "sha512-ceTwaxc4I5IOi97DgCotl3pqiyRGvffcc0oOsE2dQYaJOFIDsDt4VWG6xEbg1QePv9QWausCEIppud/tJ1wNig=="], @@ -971,6 +974,8 @@ "ast-v8-to-istanbul": ["ast-v8-to-istanbul@1.0.4", "", { "dependencies": { "@jridgewell/trace-mapping": "^0.3.31", "estree-walker": "^3.0.3", "js-tokens": "^10.0.0" } }, "sha512-0bC0/4bTSrnwdhU3IsZDwEdojvuPrSg59OYZfKsLRtJZ0u8VBx9DebfqqG8bRdCC0I7vjgxmPi41P0lpkhJHtA=="], + "atomic-sleep": ["atomic-sleep@1.0.0", "", {}, "sha512-kNOjDqAh7px0XWNI+4QbzoiR/nTkHAWNud2uvnJquD1/x5a7EQZMJT0AczqK0Qn67oY/TTQ1LbUKajZpp3I9tQ=="], + "atomically": ["atomically@2.1.1", "", { "dependencies": { "stubborn-fs": "^2.0.0", "when-exit": "^2.1.4" } }, "sha512-P4w9o2dqARji6P7MHprklbfiArZAWvo07yW7qs3pdljb3BWr12FIB7W+p0zJiuiVsUpRO0iZn1kFFcpPegg0tQ=="], "babel-dead-code-elimination": ["babel-dead-code-elimination@1.0.12", "", { "dependencies": { "@babel/core": "^7.23.7", "@babel/parser": "^7.23.6", "@babel/traverse": "^7.23.7", "@babel/types": "^7.23.6" } }, "sha512-GERT7L2TiYcYDtYk1IpD+ASAYXjKbLTDPhBtYj7X1NuRMDTMtAx9kyBenub1Ev41lo91OHCKdmP+egTDmfQ7Ig=="], @@ -1529,6 +1534,8 @@ "ohash": ["ohash@2.0.11", "", {}, "sha512-RdR9FQrFwNBNXAr4GixM8YaRZRJ5PUWbKYbE5eOsrwAjJW0q2REGcf79oYPsLyskQCZG1PLN+S/K1V00joZAoQ=="], + "on-exit-leak-free": ["on-exit-leak-free@2.1.2", "", {}, "sha512-0eJJY6hXLGf1udHwfNftBqH+g73EU4B504nZeKpz1sYRKafAghwxEJunB2O7rDZkL4PGfsMVnTXZ2EjibbqcsA=="], + "onetime": ["onetime@7.0.0", "", { "dependencies": { "mimic-function": "^5.0.0" } }, "sha512-VXJjc87FScF88uafS3JllDgvAm+c/Slfz06lorj2uAY34rlUu0Nt+v8wreiImcrgAjjIHp1rXpTDlLOGw29WwQ=="], "oniguruma-parser": ["oniguruma-parser@0.12.2", "", {}, "sha512-6HVa5oIrgMC6aA6WF6XyyqbhRPJrKR02L20+2+zpDtO5QAzGHAUGw5TKQvwi5vctNnRHkJYmjAhRVQF2EKdTQw=="], @@ -1573,6 +1580,12 @@ "picospinner": ["picospinner@3.0.0", "", {}, "sha512-lGA1TNsmy2bxvRsTI2cV01kfTwKzZjnZSDmF9llYNyMHMrU4sP87lQ5taiIKm88L3cbswjl008nwyGc3WpNvzg=="], + "pino": ["pino@10.3.1", "", { "dependencies": { "@pinojs/redact": "^0.4.0", "atomic-sleep": "^1.0.0", "on-exit-leak-free": "^2.1.0", "pino-abstract-transport": "^3.0.0", "pino-std-serializers": "^7.0.0", "process-warning": "^5.0.0", "quick-format-unescaped": "^4.0.3", "real-require": "^0.2.0", "safe-stable-stringify": "^2.3.1", "sonic-boom": "^4.0.1", "thread-stream": "^4.0.0" }, "bin": { "pino": "bin.js" } }, "sha512-r34yH/GlQpKZbU1BvFFqOjhISRo1MNx1tWYsYvmj6KIRHSPMT2+yHOEb1SG6NMvRoHRF0a07kCOox/9yakl1vg=="], + + "pino-abstract-transport": ["pino-abstract-transport@3.0.0", "", { "dependencies": { "split2": "^4.0.0" } }, "sha512-wlfUczU+n7Hy/Ha5j9a/gZNy7We5+cXp8YL+X+PG8S0KXxw7n/JXA3c46Y0zQznIJ83URJiwy7Lh56WLokNuxg=="], + + "pino-std-serializers": ["pino-std-serializers@7.1.0", "", {}, "sha512-BndPH67/JxGExRgiX1dX0w1FvZck5Wa4aal9198SrRhZjH3GxKQUKIBnYJTdj2HDN3UQAS06HlfcSbQj2OHmaw=="], + "playwright": ["playwright@1.61.0", "", { "dependencies": { "playwright-core": "1.61.0" }, "optionalDependencies": { "fsevents": "2.3.2" }, "bin": { "playwright": "cli.js" } }, "sha512-Z+7BeeqQPRRzklHsVFP4KTGIyMxKUmfeRA4WisM6G3/XW6nwGeX6fX9qYaDa+CiUqpOkb2f6X3nar05R3kSuJQ=="], "playwright-core": ["playwright-core@1.61.0", "", { "bin": { "playwright-core": "cli.js" } }, "sha512-caX7TrY3Ml6egyDX0WUcTHDxodl/b51y5wJOdCEA36QviK/s2g081hvmGs8eaE3DWb6NYZQ6BjO/QkNRPenoPA=="], @@ -1595,12 +1608,16 @@ "prismjs": ["prismjs@1.30.0", "", {}, "sha512-DEvV2ZF2r2/63V+tK8hQvrR2ZGn10srHbXviTlcv7Kpzw8jWiNTqbVgjO3IY8RxrrOUF8VPMQQFysYYYv0YZxw=="], + "process-warning": ["process-warning@5.0.0", "", {}, "sha512-a39t9ApHNx2L4+HBnQKqxxHNs1r7KF+Intd8Q/g1bUh6q0WIp9voPXJ/x0j+ZL45KF1pJd9+q2jLIRMfvEshkA=="], + "prompts": ["prompts@2.4.2", "", { "dependencies": { "kleur": "^3.0.3", "sisteransi": "^1.0.5" } }, "sha512-NxNv/kLguCA7p3jE8oL2aEBsrJWgAakBpgmgK6lpPWV+WuOmY6r2/zbAVnP+T8bQlA0nzHXSJSJW0Hq7ylaD2Q=="], "property-information": ["property-information@7.2.0", "", {}, "sha512-IAtzIB6sUiWaJYrX9smp3V46pBGbBeLFRGdh25kg1334VcBlD8HzhPeNIWQH9zhGmo2itIe25EHt9dQP7G5hmg=="], "punycode": ["punycode@2.3.1", "", {}, "sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg=="], + "quick-format-unescaped": ["quick-format-unescaped@4.0.4", "", {}, "sha512-tYC1Q1hgyRuHgloV/YXs2w15unPVh8qfu/qCTfhTYamaw7fyhumKa2yGpdSo87vY32rIclj+4fWYQXUMs9EHvg=="], + "react": ["react@19.2.7", "", {}, "sha512-HNe9WslTbXmFK8o8cmwgAeJFSBvt1bPdHCVKtaaV+WlAN36mpT4hcRpwbf3fY56ar2oIXzsBpOAiIRHAdY0OlQ=="], "react-dom": ["react-dom@19.2.7", "", { "dependencies": { "scheduler": "^0.27.0" }, "peerDependencies": { "react": "^19.2.7" } }, "sha512-t0BRVXvbiE/o20Hfw669rLbMCDWtYZLvmJigy2f0MxsXF+71pxhR3xOkspmsO8h3ZlNzyibAmtCa3l4lYKk6gQ=="], @@ -1619,6 +1636,8 @@ "readdirp": ["readdirp@4.1.2", "", {}, "sha512-GDhwkLfywWL2s6vEjyhri+eXmfH6j1L7JE27WhqLeYzoh/A3DBaYGEj2H/HFZCn/kMfim73FXxEJTw06WtxQwg=="], + "real-require": ["real-require@0.2.0", "", {}, "sha512-57frrGM/OCTLqLOAh0mhVA9VBMHd+9U7Zb2THMGdBUoZVOtGbJzjxsYGDJ3A9AYYCP4hn6y1TVbaOfzWtm5GFg=="], + "regex": ["regex@6.1.0", "", { "dependencies": { "regex-utilities": "^2.3.0" } }, "sha512-6VwtthbV4o/7+OaAF9I5L5V3llLEsoPyq9P1JVXkedTP33c7MfCG0/5NOPcSJn0TzXcG9YUrR0gQSWioew3LDg=="], "regex-recursion": ["regex-recursion@6.0.2", "", { "dependencies": { "regex-utilities": "^2.3.0" } }, "sha512-0YCaSCq2VRIebiaUviZNs0cBz1kg5kVS2UKUfNIx8YVs1cN3AV7NTctO5FOKBA+UT2BPJIWZauYHPqJODG50cg=="], @@ -1653,6 +1672,8 @@ "rw": ["rw@1.3.3", "", {}, "sha512-PdhdWy89SiZogBLaw42zdeqtRJ//zFd2PgQavcICDUgJT5oW10QCRKbJ6bg4r0/UY2M6BWd5tkxuGFRvCkgfHQ=="], + "safe-stable-stringify": ["safe-stable-stringify@2.5.0", "", {}, "sha512-b3rppTKm9T+PsVCBEOUR46GWI7fdOs00VKZ1+9c1EWDaDMvjQc6tUwuFyIprgGgTcWoVHSKrU8H31ZHA2e0RHA=="], + "safer-buffer": ["safer-buffer@2.1.2", "", {}, "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg=="], "saxes": ["saxes@6.0.0", "", { "dependencies": { "xmlchars": "^2.2.0" } }, "sha512-xAg7SOnEhrm5zI3puOOKyy1OMcMlIJZYNJY7xLBwSze0UjhPLnWfj2GF2EpT0jmzaJKIWKHLsaSSajf35bcYnA=="], @@ -1687,6 +1708,8 @@ "socket.io-parser": ["socket.io-parser@4.2.6", "", { "dependencies": { "@socket.io/component-emitter": "~3.1.0", "debug": "~4.4.1" } }, "sha512-asJqbVBDsBCJx0pTqw3WfesSY0iRX+2xzWEWzrpcH7L6fLzrhyF8WPI8UaeM4YCuDfpwA/cgsdugMsmtz8EJeg=="], + "sonic-boom": ["sonic-boom@4.2.1", "", { "dependencies": { "atomic-sleep": "^1.0.0" } }, "sha512-w6AxtubXa2wTXAUsZMMWERrsIRAdrK0Sc+FUytWvYAhBJLyuI4llrMIC1DtlNSdI99EI86KZum2MMq3EAZlF9Q=="], + "sonner": ["sonner@2.0.7", "", { "peerDependencies": { "react": "^18.0.0 || ^19.0.0 || ^19.0.0-rc", "react-dom": "^18.0.0 || ^19.0.0 || ^19.0.0-rc" } }, "sha512-W6ZN4p58k8aDKA4XPcx2hpIQXBRAgyiWVkYhT7CvK6D3iAu7xjvVyhQHg2/iaKJZ1XVJ4r7XuwGL+WGEK37i9w=="], "source-map": ["source-map@0.7.6", "", {}, "sha512-i5uvt8C3ikiWeNZSVZNWcfZPItFQOsYTUAOkcUPGd8DqDy1uOUikjt5dG+uRlwyvR108Fb9DOd4GvXfT0N2/uQ=="], @@ -1695,6 +1718,8 @@ "space-separated-tokens": ["space-separated-tokens@2.0.2", "", {}, "sha512-PEGlAwrG8yXGXRjW32fGbg66JAlOAwbObuqVoJpv/mRgoWDQfgH1wDPvtzWyUSNAXBGSk8h755YDbbcEy3SH2Q=="], + "split2": ["split2@4.2.0", "", {}, "sha512-UcjcJOWknrNkF6PLX83qcHM6KHgVKNkV62Y8a5uYDVv9ydGQVwAHMKqHdJje1VTWpljG0WYpCDhrCdAOYH4TWg=="], + "srvx": ["srvx@0.11.16", "", { "bin": { "srvx": "bin/srvx.mjs" } }, "sha512-bp07zRuycfTY43IjAvvTFnmnJi8ikW0VFiHwOhhYcVW/L4xQ1XY4PAd4Nuum1rsA17C39zL7x+CDhrn5AL32Rw=="], "stackback": ["stackback@0.0.2", "", {}, "sha512-1XMJE5fQo1jGH6Y/7ebnwPOBEkIEnT4QF32d5R1+VXdXveM0IBMJt8zfaxX1P3QhVwrYe+576+jkANtSS2mBbw=="], @@ -1739,6 +1764,8 @@ "tapable": ["tapable@2.3.3", "", {}, "sha512-uxc/zpqFg6x7C8vOE7lh6Lbda8eEL9zmVm/PLeTPBRhh1xCgdWaQ+J1CUieGpIfm2HdtsUpRv+HshiasBMcc6A=="], + "thread-stream": ["thread-stream@4.2.0", "", { "dependencies": { "real-require": "^1.0.0" } }, "sha512-e2zZ96wSChazBsbENf/Pcm/4swHt2cEKQ92rhUjkL9GCKiTDJIaTBenjE/m9DXi0QBmTMDkFDdOomUy20A1tDQ=="], + "tinybench": ["tinybench@2.9.0", "", {}, "sha512-0+DUvqWMValLmha6lr4kD8iAMK1HzV0/aKnCtWb9v9641TnP/MFb7Pc2bxoxQjTXAErryXVgUOfv2YqNllqGeg=="], "tinyexec": ["tinyexec@1.2.4", "", {}, "sha512-SHf/r48b7vOrjve9PxJo3MN5v5yuyjHvdUcrQffT3WXMUfnGmHDVbC4k3sHJaJTgZCwpUplIaAo5ANtMyp3YHg=="], @@ -1959,6 +1986,8 @@ "strip-ansi/ansi-regex": ["ansi-regex@6.2.2", "", {}, "sha512-Bq3SmSpyFHaWjPk8If9yc6svM8c56dB5BAtW4Qbw5jHTwwXXcTLoRMkpDJp6VL0XzlWaCHTXrkFURMYmD0sLqg=="], + "thread-stream/real-require": ["real-require@1.0.0", "", {}, "sha512-P4nbQYQfePJxRSmY+v/KINxVucm4NF3p3s7pJveMTtom52FR4YGltUQLB8idDXwDDWW+eYrWDFbuzUnjoWHF7g=="], + "vite/rolldown": ["rolldown@1.0.3", "", { "dependencies": { "@oxc-project/types": "=0.133.0", "@rolldown/pluginutils": "^1.0.0" }, "optionalDependencies": { "@rolldown/binding-android-arm64": "1.0.3", "@rolldown/binding-darwin-arm64": "1.0.3", "@rolldown/binding-darwin-x64": "1.0.3", "@rolldown/binding-freebsd-x64": "1.0.3", "@rolldown/binding-linux-arm-gnueabihf": "1.0.3", "@rolldown/binding-linux-arm64-gnu": "1.0.3", "@rolldown/binding-linux-arm64-musl": "1.0.3", "@rolldown/binding-linux-ppc64-gnu": "1.0.3", "@rolldown/binding-linux-s390x-gnu": "1.0.3", "@rolldown/binding-linux-x64-gnu": "1.0.3", "@rolldown/binding-linux-x64-musl": "1.0.3", "@rolldown/binding-openharmony-arm64": "1.0.3", "@rolldown/binding-wasm32-wasi": "1.0.3", "@rolldown/binding-win32-arm64-msvc": "1.0.3", "@rolldown/binding-win32-x64-msvc": "1.0.3" }, "bin": { "rolldown": "./bin/cli.mjs" } }, "sha512-i00lAJ2ks1BYr7rjNjKC7BcqAS7nVfiT3QX1SI5aY+AFHblCmaUf9OE9dbdzDvW6dJxbi2ZCZiy9v3CcwOiX3g=="], "@babel/helper-module-imports/@babel/traverse/@babel/parser": ["@babel/parser@7.29.7", "", { "dependencies": { "@babel/types": "^7.29.7" }, "bin": "./bin/babel-parser.js" }, "sha512-hnORnjP/1P/zFEndoeX+n+t1RwWRJiJpM/jO7FW32Kn9r5+sJB2JWOdYo4L6k78j15eCwY3Gm/7364B1EMwtNg=="], diff --git a/convex/lib/workerTextRedaction.ts b/convex/lib/workerTextRedaction.ts new file mode 100644 index 00000000..b8fb3f32 --- /dev/null +++ b/convex/lib/workerTextRedaction.ts @@ -0,0 +1,22 @@ +const WORKER_SECRET_VALUE_PATTERN_SOURCE = String.raw`(?:\[\s*(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\]\s"',}]+)(?:\s*,\s*(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\]\s"',}]+))*\s*\]|"[^"\r\n]*"|'[^'\r\n]*'|[^\s"',}]+)`; +const WORKER_SECRET_KEY_VALUE_PATTERN = new RegExp( + String.raw`\b([A-Z0-9_]*(?:TOKEN|SECRET|PASSWORD|API[_-]?KEY|[_-]KEY|AUTHORIZATION|CREDENTIAL)[A-Z0-9_]*|token|secret|password|api[_-]?key|authorization|credential)(["']?\s*[:=]\s*)${WORKER_SECRET_VALUE_PATTERN_SOURCE}`, + "gi", +); + +export function redactWorkerSignedUrlsAndAuthHeaders(value: string) { + return value + .replace(/https?:\/\/[^\s"')<>]+/g, "[redacted-url]") + .replace( + /\bAuthorization\s*:\s*(?:Bearer|Basic|Token)\s+[A-Za-z0-9._~+/=-]+/gi, + "[redacted-secret]", + ) + .replace(/\b(?:Bearer|Basic)\s+[A-Za-z0-9._~+/=-]+/gi, "[redacted-secret]"); +} + +export function redactWorkerPublicText(value: string) { + return redactWorkerSignedUrlsAndAuthHeaders(value).replace( + WORKER_SECRET_KEY_VALUE_PATTERN, + (_match, key: string, separator: string) => `${key}${separator}[redacted-secret]`, + ); +} diff --git a/convex/securityScan.test.ts b/convex/securityScan.test.ts index 973b620c..a6f09bcf 100644 --- a/convex/securityScan.test.ts +++ b/convex/securityScan.test.ts @@ -10,6 +10,7 @@ import { enqueueBulkSkillRescanBatchForAdminInternal, enqueueSkillVersionScanInternal, failCodexScanJob, + failJobInternal, finalizeGitHubSkillScanRequestInternal, getJobTargetInternal, getBulkSkillRescanBatchStatusForAdminInternal, @@ -17,6 +18,8 @@ import { getStoredScanReportForUserInternal, prepareGitHubSkillScanRequestInternal, pruneExpiredSkillScanRequestsInternal, + recordGitHubSkillScanResultInternal, + recordSkillScanRequestFailedInternal, requestPackageRescanForUserInternal, requestPackageRescan, requestSkillRescanForUserInternal, @@ -53,6 +56,110 @@ const failCodexScanJobHandler = ( > )._handler; +const failJobInternalHandler = ( + failJobInternal as unknown as WrappedHandler< + { jobId: string; leaseToken: string; error: string }, + { ok: true; retry: boolean } + > +)._handler; + +const recordSkillScanRequestFailedInternalHandler = ( + recordSkillScanRequestFailedInternal as unknown as WrappedHandler< + { scanId: string; error: string; llmAnalysis?: { status: string; checkedAt: number } }, + { ok: true } + > +)._handler; + +const recordGitHubSkillScanResultInternalHandler = ( + recordGitHubSkillScanResultInternal as unknown as WrappedHandler< + { + githubSkillScanId: string; + scanStatus: "clean" | "suspicious" | "malicious" | "pending" | "failed"; + error?: string; + }, + { ok: true; skipped?: string } + > +)._handler; + +function fakeLeakyWorkerError() { + return ( + `Download failed 403: https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123 ` + + `Authorization: Bearer abc OPENAI_API_KEY=openai-runtime-secret ` + + `GITHUB_TOKEN=github-runtime-secret CONVEX_DEPLOY_KEY=convex-deploy-secret ` + + `api_key=plugin-api-token sha256=${"a".repeat(64)}` + ); +} + +function expectNoLeakedWorkerErrorSecrets(error: string) { + expect(error).toContain("Download failed 403"); + expect(error).not.toContain("https://"); + expect(error).not.toContain("signed.example.invalid"); + expect(error).not.toContain("token=secret"); + expect(error).not.toContain("X-Amz-Signature"); + expect(error).not.toContain("Authorization"); + expect(error).not.toContain("Bearer abc"); + expect(error).not.toContain("openai-runtime-secret"); + expect(error).not.toContain("github-runtime-secret"); + expect(error).not.toContain("convex-deploy-secret"); + expect(error).not.toContain("plugin-api-token"); + expect(error).toContain("OPENAI_API_KEY=[redacted-secret]"); + expect(error).toContain("GITHUB_TOKEN=[redacted-secret]"); + expect(error).toContain("CONVEX_DEPLOY_KEY=[redacted-secret]"); + expect(error).toContain("api_key=[redacted-secret]"); +} + +function makeFailurePersistenceCtx(docs: Record>) { + const records = new Map>(Object.entries(docs)); + const patches: Array<{ id: string; patch: Record }> = []; + const get = vi.fn(async (id: string) => records.get(id) ?? null); + const insert = vi.fn(async (table: string, doc: Record) => { + const id = `${table}:inserted-${records.size + 1}`; + records.set(id, { _id: id, ...doc }); + return id; + }); + const patch = vi.fn(async (id: string, next: Record) => { + patches.push({ id, patch: next }); + const doc = records.get(id); + if (!doc) return; + for (const [key, value] of Object.entries(next)) { + if (value === undefined) delete doc[key]; + else doc[key] = value; + } + }); + const replace = vi.fn(async (id: string, doc: Record) => { + records.set(id, { _id: id, ...doc }); + }); + const deleteDoc = vi.fn(async (id: string) => { + records.delete(id); + }); + const query = vi.fn(() => ({ + withIndex: vi.fn(() => ({ + collect: vi.fn(async () => []), + take: vi.fn(async () => []), + unique: vi.fn(async () => null), + })), + })); + const normalizeId = vi.fn((table: string, id: string) => + id.startsWith(`${table}:`) ? id : null, + ); + return { + ctx: { + db: { + get, + insert, + patch, + query, + replace, + delete: deleteDoc, + normalizeId, + system: {}, + }, + }, + patches, + records, + }; +} + const completeCodexScanJobHandler = ( completeCodexScanJob as unknown as WrappedHandler< { @@ -2914,7 +3021,7 @@ describe("securityScan", () => { jobId: "securityScanJobs:1", leaseToken: "lease-token", error: - "Download failed https://signed.example.invalid/file?token=secret Authorization: Bearer sk-short-secret OPENAI_API_KEY=sk-short-secret", + "Download failed https://signed.example.invalid/file?token=secret Authorization: Bearer auth-secret", }, ); @@ -2945,7 +3052,7 @@ describe("securityScan", () => { | undefined; expect(llmAnalysis?.findings).toContain("Worker error"); expect(llmAnalysis?.findings).not.toContain("token=secret"); - expect(llmAnalysis?.findings).not.toContain("sk-short-secret"); + expect(llmAnalysis?.findings).not.toContain("Bearer auth-secret"); }); it("completes skill scans without directly enqueueing duplicate Skill Card jobs", async () => { @@ -3467,6 +3574,199 @@ describe("securityScan", () => { ); }); + it("sanitizes worker errors before patching failed job and scan request records", async () => { + const { ctx, records } = makeFailurePersistenceCtx({ + "securityScanJobs:1": { + _id: "securityScanJobs:1", + attempts: 3, + leaseToken: "lease-token", + nextRunAt: 123, + skillScanRequestId: "skillScanRequests:1", + status: "running", + targetKind: "skillScanRequest", + }, + "skillScanRequests:1": { + _id: "skillScanRequests:1", + status: "running", + }, + }); + + const result = await failJobInternalHandler(ctx, { + jobId: "securityScanJobs:1", + leaseToken: "lease-token", + error: fakeLeakyWorkerError(), + }); + + expect(result).toEqual({ ok: true, retry: false }); + const jobError = String(records.get("securityScanJobs:1")?.lastError); + const requestError = String(records.get("skillScanRequests:1")?.lastError); + expectNoLeakedWorkerErrorSecrets(jobError); + expectNoLeakedWorkerErrorSecrets(requestError); + }); + + it("sanitizes worker errors before patching failed scan result records", async () => { + const { ctx, records } = makeFailurePersistenceCtx({ + "githubSkillScans:1": { + _id: "githubSkillScans:1", + contentHash: "content-hash", + skillId: "skills:missing", + status: "pending", + }, + "skillScanRequests:1": { + _id: "skillScanRequests:1", + status: "running", + }, + }); + + await expect( + recordSkillScanRequestFailedInternalHandler(ctx, { + scanId: "skillScanRequests:1", + error: fakeLeakyWorkerError(), + }), + ).resolves.toEqual({ ok: true }); + + await expect( + recordGitHubSkillScanResultInternalHandler(ctx, { + githubSkillScanId: "githubSkillScans:1", + scanStatus: "failed", + error: fakeLeakyWorkerError(), + }), + ).resolves.toMatchObject({ ok: true }); + + const requestError = String(records.get("skillScanRequests:1")?.lastError); + const githubScanError = String(records.get("githubSkillScans:1")?.lastError); + expectNoLeakedWorkerErrorSecrets(requestError); + expectNoLeakedWorkerErrorSecrets(githubScanError); + }); + + it("redacts signed artifact URLs from persisted worker failure fields", async () => { + vi.stubEnv("SECURITY_SCAN_WORKER_TOKEN", "worker-secret"); + + const leakyError = + `Download failed 403: https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123 ` + + `Authorization: Bearer abc OPENAI_API_KEY=openai-runtime-secret ` + + `GITHUB_TOKEN=github-runtime-secret CONVEX_DEPLOY_KEY=convex-deploy-secret ` + + "api_key=plugin-api-token"; + const runMutation = vi.fn(async (_ref: unknown, args: Record) => + "error" in args ? { ok: true, retry: false } : { ok: true }, + ); + const runQuery = vi.fn(async () => ({ + job: { + _id: "securityScanJobs:1", + targetKind: "skillScanRequest", + }, + scanRequest: { + _id: "skillScanRequests:1", + sourceKind: "github", + githubSkillScanId: "githubSkillScans:1", + }, + githubScan: { + _id: "githubSkillScans:1", + skillId: "skills:1", + contentHash: "content-hash", + }, + })); + + await failCodexScanJobHandler( + { runQuery, runMutation }, + { + token: "worker-secret", + jobId: "securityScanJobs:1", + leaseToken: "lease-token", + error: leakyError, + }, + ); + + const persistedErrorArgs = runMutation.mock.calls + .map(([, args]) => args) + .filter((args): args is Record => { + return typeof args === "object" && args !== null && "error" in args; + }); + expect(persistedErrorArgs).toHaveLength(3); + for (const args of persistedErrorArgs) { + expect(args.error).toBeTypeOf("string"); + const error = String(args.error); + expect(error).toContain("Download failed 403"); + expect(error).not.toContain("https://"); + expect(error).not.toContain("signed.example.invalid"); + expect(error).not.toContain("token=secret"); + expect(error).not.toContain("X-Amz-Signature"); + expect(error).not.toContain("Authorization"); + expect(error).not.toContain("Bearer abc"); + expect(error).not.toContain("openai-runtime-secret"); + expect(error).not.toContain("github-runtime-secret"); + expect(error).not.toContain("convex-deploy-secret"); + expect(error).not.toContain("plugin-api-token"); + expect(error).toContain("OPENAI_API_KEY=[redacted-secret]"); + expect(error).toContain("GITHUB_TOKEN=[redacted-secret]"); + expect(error).toContain("CONVEX_DEPLOY_KEY=[redacted-secret]"); + expect(error).toContain("api_key=[redacted-secret]"); + } + + const llmAnalyses = runMutation.mock.calls + .map(([, args]) => { + if (!args || typeof args !== "object" || !("llmAnalysis" in args)) return undefined; + const analysis = args.llmAnalysis; + if (!analysis || typeof analysis !== "object" || !("findings" in analysis)) { + return undefined; + } + return typeof analysis.findings === "string" ? analysis : undefined; + }) + .filter((analysis) => analysis !== undefined); + for (const analysis of llmAnalyses) { + expect(analysis?.findings).toContain("Download failed 403"); + expect(analysis?.findings).not.toContain("https://"); + expect(analysis?.findings).not.toContain("signed.example.invalid"); + expect(analysis?.findings).not.toContain("token=secret"); + expect(analysis?.findings).not.toContain("X-Amz-Signature"); + expect(analysis?.findings).not.toContain("Authorization"); + expect(analysis?.findings).not.toContain("Bearer abc"); + expect(analysis?.findings).not.toContain("openai-runtime-secret"); + expect(analysis?.findings).not.toContain("github-runtime-secret"); + expect(analysis?.findings).not.toContain("convex-deploy-secret"); + expect(analysis?.findings).not.toContain("plugin-api-token"); + expect(analysis?.findings).toContain("OPENAI_API_KEY=[redacted-secret]"); + expect(analysis?.findings).toContain("GITHUB_TOKEN=[redacted-secret]"); + expect(analysis?.findings).toContain("CONVEX_DEPLOY_KEY=[redacted-secret]"); + expect(analysis?.findings).toContain("api_key=[redacted-secret]"); + } + }); + + it("redacts signed artifact URLs from package failure analysis fields", async () => { + vi.stubEnv("SECURITY_SCAN_WORKER_TOKEN", "worker-secret"); + + const runMutation = vi.fn(async (_ref: unknown, args: Record) => + "error" in args ? { ok: true, retry: false } : { ok: true }, + ); + const runQuery = vi.fn(async () => ({ + job: { + _id: "securityScanJobs:1", + targetKind: "packageRelease", + }, + release: { + _id: "packageReleases:1", + }, + })); + + await failCodexScanJobHandler( + { runMutation, runQuery }, + { + token: "worker-secret", + jobId: "securityScanJobs:1", + leaseToken: "lease-token", + error: fakeLeakyWorkerError(), + }, + ); + + const packageFailureCall = runMutation.mock.calls.find(([, args]) => { + return args && typeof args === "object" && "releaseId" in args && "llmAnalysis" in args; + }); + expect(packageFailureCall).toBeDefined(); + const llmAnalysis = packageFailureCall?.[1].llmAnalysis as { findings?: string } | undefined; + expect(llmAnalysis?.findings).toBeTypeOf("string"); + expectNoLeakedWorkerErrorSecrets(llmAnalysis?.findings ?? ""); + }); + it("preserves a prior blocking skill ClawScan verdict when worker retries are exhausted", async () => { vi.stubEnv("SECURITY_SCAN_WORKER_TOKEN", "worker-secret"); diff --git a/convex/securityScan.ts b/convex/securityScan.ts index 4b5fafd9..2d53703c 100644 --- a/convex/securityScan.ts +++ b/convex/securityScan.ts @@ -20,6 +20,7 @@ import { MAX_SKILL_SCAN_REQUEST_MANIFEST_BYTES, serializedSkillScanRequestFilesBytes, } from "./lib/skillScanRequestFiles"; +import { redactWorkerPublicText } from "./lib/workerTextRedaction"; const DEFAULT_VT_WAIT_MS = 10 * 60 * 1000; const DEFAULT_LEASE_MS = 60 * 60 * 1000; @@ -348,23 +349,13 @@ function githubSkillScanStatusFromLlmAnalysis( return "failed" as const; } +function sanitizeWorkerErrorDetail(error: string, maxChars = 500) { + const redacted = redactWorkerPublicText(error); + return redacted.slice(0, maxChars); +} + function publicWorkerErrorDetail(error: string) { - return error - .replace(/https?:\/\/[^\s"')<>]+/g, "[redacted-url]") - .replace( - /\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]{12,}/gi, - (_match, scheme: string) => `${scheme} [redacted-secret]`, - ) - .replace( - /\b(token|secret|password|api[_-]?key|authorization)(["']?\s*[:=]\s*["']?)[^\s"',}]+/gi, - (_match, key: string, separator: string) => `${key}${separator}[redacted-secret]`, - ) - .replace( - /\b([A-Z0-9_]*(?:TOKEN|SECRET|PASSWORD|API[_-]?KEY|AUTHORIZATION))(["']?\s*[:=]\s*["']?)[^\s"',}]+/gi, - (_match, key: string, separator: string) => `${key}${separator}[redacted-secret]`, - ) - .replace(/\b[A-Za-z0-9_+/=-]{64,}\b/g, "[redacted-secret]") - .slice(0, 500); + return sanitizeWorkerErrorDetail(error, 500); } function truncateSkillSpectorStorageText( @@ -1793,9 +1784,10 @@ export const recordSkillScanRequestFailedInternal = internalMutation({ const request = await ctx.db.get(args.scanId); if (!request) throw new ConvexError("Scan request not found"); const now = Date.now(); + const error = sanitizeWorkerErrorDetail(args.error, 2000); await ctx.db.patch(request._id, { status: "failed", - lastError: args.error.slice(0, 2000), + lastError: error, ...(args.llmAnalysis ? { llmAnalysis: args.llmAnalysis } : {}), completedAt: now, updatedAt: now, @@ -1817,11 +1809,12 @@ export const recordGitHubSkillScanResultInternal = internalMutation({ const scan = await ctx.db.get(args.githubSkillScanId); if (!scan) return { ok: true as const, skipped: "missing-scan" as const }; const now = Date.now(); + const error = args.error ? sanitizeWorkerErrorDetail(args.error, 2000) : undefined; await ctx.db.patch(scan._id, { status: args.scanStatus, llmAnalysis: args.llmAnalysis, skillSpectorAnalysis: args.skillSpectorAnalysis, - lastError: args.error?.slice(0, 2000), + lastError: error, runId: args.runId, completedAt: now, updatedAt: now, @@ -2469,9 +2462,10 @@ export const failJobInternal = internalMutation({ if (!job || job.leaseToken !== args.leaseToken) throw new ConvexError("Lease mismatch"); const now = Date.now(); const retry = job.attempts < MAX_ATTEMPTS; + const error = sanitizeWorkerErrorDetail(args.error, 2000); await ctx.db.patch(args.jobId, { status: retry ? "queued" : "failed", - lastError: args.error.slice(0, 2000), + lastError: error, nextRunAt: retry ? now + Math.min(30 * 60 * 1000, 2 ** job.attempts * 60_000) : job.nextRunAt, leaseToken: undefined, leaseExpiresAt: undefined, @@ -2481,7 +2475,7 @@ export const failJobInternal = internalMutation({ if (job.targetKind === "skillScanRequest" && job.skillScanRequestId) { await ctx.db.patch(job.skillScanRequestId, { status: retry ? "queued" : "failed", - lastError: args.error.slice(0, 2000), + lastError: error, ...(retry ? {} : { completedAt: now }), updatedAt: now, }); @@ -2707,13 +2701,14 @@ export const failCodexScanJob = action({ }, handler: async (ctx, args) => { assertWorkerToken(args.token); + const error = sanitizeWorkerErrorDetail(args.error, 2000); const result = await runMutationRef<{ ok: true; retry: boolean }>( ctx, internalRefs.securityScan.failJobInternal, { jobId: args.jobId, leaseToken: args.leaseToken, - error: args.error, + error, }, ); @@ -2726,7 +2721,7 @@ export const failCodexScanJob = action({ }, ); if (target && !target.missing) { - const llmAnalysis = buildWorkerFailureLlmAnalysis(args.error); + const llmAnalysis = buildWorkerFailureLlmAnalysis(error); if (target.job.targetKind === "skillVersion" && target.version) { if (!hasArtifactBackedLlmAnalysis(target.version.llmAnalysis)) { await runMutationRef(ctx, internalRefs.skills.updateVersionLlmAnalysisInternal, { @@ -2750,7 +2745,7 @@ export const failCodexScanJob = action({ { githubSkillScanId: target.githubScan._id, scanStatus: "failed", - error: args.error, + error, llmAnalysis, }, ); @@ -2760,7 +2755,7 @@ export const failCodexScanJob = action({ internalRefs.securityScan.recordSkillScanRequestFailedInternal, { scanId: target.scanRequest._id, - error: args.error, + error, llmAnalysis, }, ); diff --git a/convex/skillCards.test.ts b/convex/skillCards.test.ts index e974f27b..e05defd5 100644 --- a/convex/skillCards.test.ts +++ b/convex/skillCards.test.ts @@ -664,6 +664,50 @@ describe("skillCards queue", () => { }), ); }); + + it("redacts worker failure details before persistence", async () => { + const patch = vi.fn(async (_id: string, _patch: Record) => undefined); + const ctx = { + db: completeDb({ + get: vi.fn(async () => ({ + _id: "skillCardGenerationJobs:1", + leaseToken: "lease", + attempts: 3, + nextRunAt: 1, + })), + patch, + }), + }; + + const result = await failHandler(ctx, { + jobId: "skillCardGenerationJobs:1", + leaseToken: "lease", + error: + "Download failed 403: https://signed.example.invalid/file?token=secret " + + "Authorization: Bearer worker-secret OPENAI_API_KEY=openai-runtime-secret " + + "path=artifacts/token=artifact-path-secret.json", + }); + + expect(result).toEqual({ ok: true, retry: false }); + expect(patch).toHaveBeenCalledWith( + "skillCardGenerationJobs:1", + expect.objectContaining({ + status: "failed", + lastError: expect.any(String), + }), + ); + const patchPayload = patch.mock.calls[0]?.[1] as { lastError?: unknown } | undefined; + const lastError = String(patchPayload?.lastError); + expect(lastError).toContain("Download failed 403"); + expect(lastError).not.toContain("https://"); + expect(lastError).not.toContain("signed.example.invalid"); + expect(lastError).not.toContain("token=secret"); + expect(lastError).not.toContain("Authorization"); + expect(lastError).not.toContain("worker-secret"); + expect(lastError).not.toContain("openai-runtime-secret"); + expect(lastError).not.toContain("artifact-path-secret"); + expect(lastError).toContain("OPENAI_API_KEY=[redacted-secret]"); + }); }); describe("skillCards attach", () => { diff --git a/convex/skillCards.ts b/convex/skillCards.ts index 9667db63..59a73271 100644 --- a/convex/skillCards.ts +++ b/convex/skillCards.ts @@ -11,6 +11,7 @@ import { SKILL_CARD_FILE_PATH, sourceSkillVersionFiles, } from "./lib/skillCards"; +import { redactWorkerPublicText } from "./lib/workerTextRedaction"; const DEFAULT_LEASE_MS = 60 * 60 * 1000; const DEFAULT_SKILL_CARD_CLAIM_LIMIT = 6; @@ -22,6 +23,10 @@ const jobSourceValidator = v.union(v.literal("publish"), v.literal("scan"), v.li type SkillCardJob = Doc<"skillCardGenerationJobs">; type SkillVersionFile = Doc<"skillVersions">["files"][number]; +function sanitizeWorkerErrorDetail(error: string, maxChars = 2000) { + return redactWorkerPublicText(error).slice(0, maxChars); +} + type SkillCardTarget = { job: SkillCardJob; skill?: Doc<"skills">; @@ -422,9 +427,10 @@ export const failJobInternal = internalMutation({ if (!job || job.leaseToken !== args.leaseToken) throw new ConvexError("Lease mismatch"); const now = Date.now(); const retry = job.attempts < MAX_ATTEMPTS; + const error = sanitizeWorkerErrorDetail(args.error); await ctx.db.patch(args.jobId, { status: retry ? "queued" : "failed", - lastError: args.error.slice(0, 2000), + lastError: error, nextRunAt: retry ? now + Math.min(30 * 60 * 1000, 2 ** job.attempts * 60_000) : job.nextRunAt, leaseToken: undefined, leaseExpiresAt: undefined, diff --git a/package.json b/package.json index 5ac18f00..28f74e64 100644 --- a/package.json +++ b/package.json @@ -109,6 +109,7 @@ "mermaid": "^11.15.0", "mime": "4.1.0", "monaco-editor": "0.55.1", + "pino": "10.3.1", "react": "19.2.7", "react-dom": "19.2.7", "react-markdown": "10.1.0", diff --git a/scripts/lib/workerLogger.test.ts b/scripts/lib/workerLogger.test.ts new file mode 100644 index 00000000..f31cd222 --- /dev/null +++ b/scripts/lib/workerLogger.test.ts @@ -0,0 +1,78 @@ +/* @vitest-environment node */ +import { describe, expect, it } from "vitest"; +import { createWorkerLogger } from "./workerLogger"; + +describe("worker logger", () => { + it("emits structured info and error log events", () => { + const lines: string[] = []; + const logger = createWorkerLogger({ + name: "worker-logger-test", + destination: { write: (line) => lines.push(line) }, + }); + + logger.info({ event: "worker_test_started", jobId: "job1" }, "started"); + logger.error({ event: "worker_test_failed", jobId: "job1" }, "failed"); + + expect(lines).toHaveLength(2); + const [info, error] = lines.map((line) => JSON.parse(line) as Record); + expect(info).toMatchObject({ + event: "worker_test_started", + jobId: "job1", + level: 30, + msg: "started", + service: "worker-logger-test", + }); + expect(error).toMatchObject({ + event: "worker_test_failed", + jobId: "job1", + level: 50, + msg: "failed", + service: "worker-logger-test", + }); + }); + + it("redacts configured structured fields in emitted JSON", () => { + const lines: string[] = []; + const logger = createWorkerLogger({ + name: "worker-logger-test", + destination: { write: (line) => lines.push(line) }, + }); + + logger.info( + { + artifact: { + downloadUrl: "https://signed.example.invalid/file?token=secret", + path: "SKILL.md", + }, + headers: { authorization: "Bearer worker-token-secret" }, + rawResult: "raw scanner output with url=https://signed.example.invalid/raw", + publicReason: "Download failed 403 for artifact file SKILL.md", + reason: "raw reason with https://signed.example.invalid/reason", + stderr: "Authorization: Basic abc123", + stdout: "raw stdout transcript", + target: { + files: [ + { + path: "package.json", + url: "https://signed.example.invalid/package?X-Amz-Signature=abc", + }, + ], + }, + token: "worker-token-secret", + }, + "structured worker event", + ); + + expect(lines).toHaveLength(1); + const parsed = JSON.parse(lines[0] ?? "") as Record; + const text = JSON.stringify(parsed); + expect(parsed.msg).toBe("structured worker event"); + expect(text).not.toContain("signed.example.invalid"); + expect(text).not.toContain("worker-token-secret"); + expect(text).not.toContain("raw reason"); + expect(text).not.toContain("raw stdout transcript"); + expect(text).toContain("Download failed 403 for artifact file SKILL.md"); + expect(text).toContain("[redacted-secret]"); + expect(parsed.artifact).toMatchObject({ path: "SKILL.md" }); + }); +}); diff --git a/scripts/lib/workerLogger.ts b/scripts/lib/workerLogger.ts new file mode 100644 index 00000000..dbdff094 --- /dev/null +++ b/scripts/lib/workerLogger.ts @@ -0,0 +1,59 @@ +import pino, { type DestinationStream, type Logger } from "pino"; + +export const WORKER_LOG_REDACTION_PATHS = [ + "token", + "secret", + "password", + "apiKey", + "api_key", + "authorization", + "Authorization", + "headers.authorization", + "headers.Authorization", + "request.headers.authorization", + "request.headers.Authorization", + "artifact.url", + "artifact.downloadUrl", + "artifact.clawpackUrl", + "artifact.signedUrl", + "artifacts[*].url", + "target.files[*].url", + "target.clawpackUrl", + "url", + "downloadUrl", + "clawpackUrl", + "signedUrl", + "error", + "reason", + "err.message", + "err.stack", + "stderr", + "stdout", + "rawResult", +] as const; + +function stdoutDestination(): DestinationStream { + return { + write(line: string) { + process.stdout.write(line); + }, + }; +} + +export function createWorkerLogger(options?: { + destination?: DestinationStream; + level?: string; + name: string; +}): Logger { + return pino( + { + base: { service: options?.name }, + level: options?.level ?? process.env.WORKER_LOG_LEVEL ?? "info", + redact: { + censor: "[redacted-secret]", + paths: [...WORKER_LOG_REDACTION_PATHS], + }, + }, + options?.destination ?? stdoutDestination(), + ); +} diff --git a/scripts/lib/workerRedaction.test.ts b/scripts/lib/workerRedaction.test.ts new file mode 100644 index 00000000..9686f25a --- /dev/null +++ b/scripts/lib/workerRedaction.test.ts @@ -0,0 +1,154 @@ +/* @vitest-environment node */ +import { describe, expect, it } from "vitest"; +import { redactWorkerSignedUrlsAndAuthHeaders } from "../../convex/lib/workerTextRedaction"; +import { + maskGitHubActionsSecret, + maskKnownWorkerSecrets, + redactWorkerPublicErrorMessage, + redactWorkerPublicText, + safeWorkerArtifactPathLabel, +} from "./workerRedaction"; + +describe("worker transport redaction", () => { + it("redacts URLs and auth headers without acting as a secret detector", () => { + const sha256 = "a".repeat(64); + const raw = [ + "download https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc", + "Authorization: Bearer abc.def.ghi", + "Authorization: Basic dXNlcjpwYXNz", + "OPENAI_API_KEY=openai-runtime-secret", + "GITHUB_TOKEN=runtime-token-secret", + "CONVEX_DEPLOY_KEY=convex-deploy-secret", + "api_key=plugin-api-token", + `artifact_sha256=${sha256}`, + ].join("\n"); + + const redacted = redactWorkerSignedUrlsAndAuthHeaders(raw); + + expect(redacted).not.toContain("https://"); + expect(redacted).not.toContain("signed.example.invalid"); + expect(redacted).not.toContain("token=secret"); + expect(redacted).not.toContain("X-Amz-Signature"); + expect(redacted).not.toContain("Bearer abc"); + expect(redacted).not.toContain("Basic dXN"); + expect(redacted).toContain("OPENAI_API_KEY=openai-runtime-secret"); + expect(redacted).toContain("GITHUB_TOKEN=runtime-token-secret"); + expect(redacted).toContain("api_key=plugin-api-token"); + expect(redacted).toContain(`artifact_sha256=${sha256}`); + expect(redacted).toContain("[redacted-url]"); + expect(redacted).toContain("[redacted-secret]"); + }); + + it("uses the same narrow cleanup for worker error messages", () => { + const message = redactWorkerSignedUrlsAndAuthHeaders( + "fetch failed https://signed.example.invalid/file Authorization: Bearer abc.def", + ); + + expect(message).not.toContain("https://"); + expect(message).not.toContain("Bearer abc"); + expect(message).toContain("[redacted-url]"); + expect(message).toContain("[redacted-secret]"); + }); + + it("redacts key-value secrets at public log and persistence boundaries", () => { + const sha256 = "a".repeat(64); + const raw = [ + "download https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc", + "Authorization: Bearer abc.def.ghi", + "Authorization: Token token-runtime-secret", + "OPENAI_API_KEY=openai-runtime-secret", + "GITHUB_TOKEN=runtime-token-secret", + "CONVEX_DEPLOY_KEY=convex-deploy-secret", + "api_key=plugin-api-token", + 'PRIVATE_KEY="secret-part-one,secret-part-two"', + 'token=["array-token-secret"]', + 'Authorization: ["array-auth-secret"]', + '{"token":["json-token-secret"],"authorization":["json-auth-secret"]}', + `artifact_sha256=${sha256}`, + ].join("\n"); + + const redacted = redactWorkerPublicText(raw); + + expect(redacted).not.toContain("https://"); + expect(redacted).not.toContain("signed.example.invalid"); + expect(redacted).not.toContain("Bearer abc"); + expect(redacted).not.toContain("token-runtime-secret"); + expect(redacted).not.toContain("openai-runtime-secret"); + expect(redacted).not.toContain("runtime-token-secret"); + expect(redacted).not.toContain("convex-deploy-secret"); + expect(redacted).not.toContain("plugin-api-token"); + expect(redacted).not.toContain("secret-part-one"); + expect(redacted).not.toContain("secret-part-two"); + expect(redacted).not.toContain("array-token-secret"); + expect(redacted).not.toContain("array-auth-secret"); + expect(redacted).not.toContain("json-token-secret"); + expect(redacted).not.toContain("json-auth-secret"); + expect(redacted).toContain("OPENAI_API_KEY=[redacted-secret]"); + expect(redacted).toContain("GITHUB_TOKEN=[redacted-secret]"); + expect(redacted).toContain("CONVEX_DEPLOY_KEY=[redacted-secret]"); + expect(redacted).toContain("api_key=[redacted-secret]"); + expect(redacted).toContain(`artifact_sha256=${sha256}`); + expect(redacted).toContain("[redacted-secret]"); + }); + + it("uses the public boundary for worker error messages that can persist", () => { + const message = redactWorkerPublicErrorMessage( + "fetch failed https://signed.example.invalid/file OPENAI_API_KEY=sk-runtime-secret", + ); + + expect(message).not.toContain("https://"); + expect(message).not.toContain("sk-runtime-secret"); + expect(message).toContain("OPENAI_API_KEY=[redacted-secret]"); + }); + + it("only displays artifact paths that pass a safe allowlist", () => { + expect(safeWorkerArtifactPathLabel("SKILL.md")).toBe("SKILL.md"); + expect(safeWorkerArtifactPathLabel("nested/package.json")).toBe("nested/package.json"); + expect(safeWorkerArtifactPathLabel("../SKILL.md")).toBe("[redacted-path]"); + expect(safeWorkerArtifactPathLabel("unsafe/token=runtime-value.md")).toBe("[redacted-path]"); + }); + + it("emits exact GitHub Actions masks only in GitHub Actions", () => { + const lines: string[] = []; + + expect( + maskGitHubActionsSecret("https://signed.example.invalid/file?token=secret", { + env: { GITHUB_ACTIONS: "true" } as NodeJS.ProcessEnv, + write: (line) => lines.push(line), + }), + ).toBe(true); + expect( + maskGitHubActionsSecret("a%b\nc\r", { + env: { GITHUB_ACTIONS: "true" } as NodeJS.ProcessEnv, + write: (line) => lines.push(line), + }), + ).toBe(true); + expect( + maskGitHubActionsSecret("local-secret", { + env: {} as NodeJS.ProcessEnv, + write: (line) => lines.push(line), + }), + ).toBe(false); + + expect(lines).toEqual([ + "::add-mask::https://signed.example.invalid/file?token=secret\n", + "::add-mask::a%25b%0Ac%0D\n", + ]); + }); + + it("masks known worker secrets from the runtime environment", () => { + const lines: string[] = []; + + maskKnownWorkerSecrets( + { + GITHUB_ACTIONS: "true", + OPENAI_API_KEY: "sk-runtime-secret", + SECURITY_SCAN_WORKER_TOKEN: "worker-token-secret", + } as NodeJS.ProcessEnv, + (line) => lines.push(line), + ); + + expect(lines).toContain("::add-mask::sk-runtime-secret\n"); + expect(lines).toContain("::add-mask::worker-token-secret\n"); + }); +}); diff --git a/scripts/lib/workerRedaction.ts b/scripts/lib/workerRedaction.ts new file mode 100644 index 00000000..046faa80 --- /dev/null +++ b/scripts/lib/workerRedaction.ts @@ -0,0 +1,60 @@ +import { redactWorkerPublicText as redactSharedWorkerPublicText } from "../../convex/lib/workerTextRedaction"; + +const DEFAULT_MAX_TEXT_CHARS = 20_000; + +export function redactWorkerPublicText(value: string, maxChars = DEFAULT_MAX_TEXT_CHARS) { + const redacted = redactSharedWorkerPublicText(value); + if (redacted.length <= maxChars) return redacted; + return `${redacted.slice(0, maxChars)}\n...[truncated ${redacted.length - maxChars} chars]`; +} + +export function redactWorkerPublicErrorMessage(value: string) { + return redactWorkerPublicText(value); +} + +export function safeWorkerArtifactPathLabel(value: string) { + const normalized = value.replace(/[\r\n]+/g, " ").trim(); + const parts = normalized.split("/"); + const redacted = redactSharedWorkerPublicText(normalized); + const isSafe = + normalized.length > 0 && + normalized.length <= 240 && + redacted === normalized && + !normalized.startsWith("/") && + !parts.includes("..") && + /^[A-Za-z0-9._/-]+$/.test(normalized); + return isSafe ? normalized : "[redacted-path]"; +} + +function escapeGitHubActionsCommandValue(value: string) { + return value.replace(/%/g, "%25").replace(/\r/g, "%0D").replace(/\n/g, "%0A"); +} + +export function maskGitHubActionsSecret( + value: string | null | undefined, + options?: { env?: NodeJS.ProcessEnv; write?: (line: string) => void }, +) { + if (!value) return false; + const env = options?.env ?? process.env; + if (env.GITHUB_ACTIONS !== "true") return false; + const write = options?.write ?? ((line: string) => process.stdout.write(line)); + write(`::add-mask::${escapeGitHubActionsCommandValue(value)}\n`); + return true; +} + +export function maskKnownWorkerSecrets( + env: NodeJS.ProcessEnv = process.env, + write?: (line: string) => void, +) { + const secretKeys = [ + "SECURITY_SCAN_WORKER_TOKEN", + "OPENAI_API_KEY", + "GH_TOKEN", + "GITHUB_TOKEN", + "CONVEX_DEPLOY_KEY", + "HOMEBREW_GITHUB_API_TOKEN", + ]; + for (const key of secretKeys) { + maskGitHubActionsSecret(env[key], { env, write }); + } +} diff --git a/scripts/security/run-codex-scan-worker.test.ts b/scripts/security/run-codex-scan-worker.test.ts index ce322c98..9c76a8d0 100644 --- a/scripts/security/run-codex-scan-worker.test.ts +++ b/scripts/security/run-codex-scan-worker.test.ts @@ -13,6 +13,7 @@ import * as codexScanWorker from "./run-codex-scan-worker"; import { buildPrompt, normalizeSkillSpectorAnalysis, + processJob, resolveSkillSpectorScanInput, writeArtifactWorkspace, writeJobDiagnostic, @@ -21,6 +22,7 @@ import { const tempDirs: string[] = []; afterEach(async () => { + vi.restoreAllMocks(); await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { force: true, recursive: true }))); }); @@ -30,6 +32,31 @@ async function tempDir() { return dir; } +async function readAllFilesText(dir: string) { + const texts: string[] = []; + async function visit(current: string) { + for (const entry of await readdir(current, { withFileTypes: true })) { + const path = join(current, entry.name); + if (entry.isDirectory()) { + await visit(path); + } else if (entry.isFile()) { + texts.push(await readFile(path, "utf8")); + } + } + } + await visit(dir); + return texts.join("\n"); +} + +function unsafeFixtureLabels() { + return { + label: ["API", "key"].join(" "), + pathSegment: ["unsafe", "label"].join("-"), + runtimeValue: "sk-short-fixture", + workerValue: "worker-token-fixture", + }; +} + describe("run-codex-scan-worker diagnostics", () => { it("keeps successful claims when a parallel claim request fails", async () => { const claimCodexScanJobBatch = ( @@ -49,7 +76,7 @@ describe("run-codex-scan-worker diagnostics", () => { target: Record; }> >, - ) => Promise>; + ) => Promise<{ claimFailures: number; jobs: Array<{ job: { _id: string } }> }>; } ).claimCodexScanJobBatch; expect(claimCodexScanJobBatch).toBeTypeOf("function"); @@ -70,16 +97,52 @@ describe("run-codex-scan-worker diagnostics", () => { target: {}, }, ]) - .mockRejectedValueOnce(new Error("temporary claim failure")) + .mockRejectedValueOnce( + new Error( + "temporary claim failure https://signed.example.invalid/file?token=claim-secret OPENAI_API_KEY=claim-process-secret", + ), + ) .mockResolvedValueOnce([]); - const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined); + const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true); - await expect(claimCodexScanJobBatch(3, claimOne)).resolves.toMatchObject([ - { job: { _id: "securityScanJobs:1" } }, - ]); + await expect(claimCodexScanJobBatch(3, claimOne)).resolves.toMatchObject({ + claimFailures: 1, + jobs: [{ job: { _id: "securityScanJobs:1" } }], + }); expect(claimOne).toHaveBeenCalledTimes(3); - expect(consoleError).toHaveBeenCalledWith(expect.stringContaining("temporary claim failure")); - consoleError.mockRestore(); + const logged = stdoutWrite.mock.calls.map((call) => String(call[0])).join(""); + expect(logged).toContain("security_scan_claim_failed"); + expect(logged).toContain("temporary claim failure"); + expect(logged).not.toContain("https://signed.example.invalid"); + expect(logged).not.toContain("claim-secret"); + expect(logged).not.toContain("claim-process-secret"); + stdoutWrite.mockRestore(); + }); + + it("counts total claim failures even when no jobs are claimed", async () => { + const claimCodexScanJobBatch = ( + codexScanWorker as typeof codexScanWorker & { + claimCodexScanJobBatch?: ( + claimLimit: number, + claimOne: () => Promise, + ) => Promise<{ claimFailures: number; jobs: Array<{ job: { _id: string } }> }>; + } + ).claimCodexScanJobBatch; + expect(claimCodexScanJobBatch).toBeTypeOf("function"); + if (!claimCodexScanJobBatch) return; + + const claimOne = vi.fn().mockRejectedValue(new Error("claim outage")); + const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + + await expect(claimCodexScanJobBatch(2, claimOne)).resolves.toMatchObject({ + claimFailures: 2, + jobs: [], + }); + expect(claimOne).toHaveBeenCalledTimes(2); + const logged = stdoutWrite.mock.calls.map((call) => String(call[0])).join(""); + expect(logged).toContain("security_scan_claim_failed"); + expect(logged).toContain("claim outage"); + stdoutWrite.mockRestore(); }); it("blocks direct local Codex security worker runs without opt-in", () => { @@ -346,6 +409,310 @@ describe("run-codex-scan-worker diagnostics", () => { expect(metadata.target.files).toEqual([{ path: "SKILL.md", sha256: "abc123", size: 42 }]); }); + it("omits signed artifact URLs from download failure errors", async () => { + const unsafeLabels = unsafeFixtureLabels(); + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response("forbidden", { status: 403 })); + const workspace = await tempDir(); + + await expect( + writeArtifactWorkspace( + { + job: { + _id: "job123", + hasMaliciousSignal: false, + leaseToken: "lease-secret", + source: "publish", + targetKind: "skillVersion", + waitForVtUntil: 0, + }, + target: { + files: [ + { + path: "SKILL.md", + sha256: "abc123", + size: 42, + url: "https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123", + }, + ], + }, + }, + workspace, + ), + ).rejects.toThrow("Download failed 403 for artifact file SKILL.md"); + + const error = await writeArtifactWorkspace( + { + job: { + _id: "job124", + hasMaliciousSignal: false, + leaseToken: "lease-secret", + source: "publish", + targetKind: "skillVersion", + waitForVtUntil: 0, + }, + target: { + files: [ + { + path: "package.json", + sha256: "def456", + size: 54, + url: "https://signed.example.invalid/package?Authorization=Bearer-secret", + }, + ], + }, + }, + await tempDir(), + ).catch((caught: unknown) => caught); + + const message = error instanceof Error ? error.message : String(error); + expect(message).not.toContain("https://"); + expect(message).not.toContain("signed.example.invalid"); + expect(message).not.toContain("token=secret"); + expect(message).not.toContain("X-Amz-Signature"); + expect(message).not.toContain("Authorization"); + + const unsafePath = + `unsafe/token=${unsafeLabels.workerValue}-api_key=${unsafeLabels.pathSegment}-` + + `X-Amz-Signature=${"a".repeat(32)}.md`; + const unsafePathError = await writeArtifactWorkspace( + { + job: { + _id: "job124b", + hasMaliciousSignal: false, + leaseToken: "lease-secret", + source: "publish", + targetKind: "skillVersion", + waitForVtUntil: 0, + }, + target: { + files: [ + { + path: unsafePath, + sha256: "unsafe-label-fixture", + size: 61, + url: "https://signed.example.invalid/package?token=secret", + }, + ], + }, + }, + await tempDir(), + ).catch((caught: unknown) => caught); + const unsafePathMessage = + unsafePathError instanceof Error ? unsafePathError.message : String(unsafePathError); + expect(unsafePathMessage).toContain("Download failed 403 for artifact file"); + expect(unsafePathMessage).not.toContain(unsafeLabels.workerValue); + expect(unsafePathMessage).not.toContain(`api_key=${unsafeLabels.pathSegment}`); + expect(unsafePathMessage).not.toContain(unsafeLabels.pathSegment); + expect(unsafePathMessage).not.toContain("X-Amz-Signature"); + + fetchMock.mockRejectedValueOnce( + new Error( + `fetch failed https://signed.example.invalid/file?token=secret Authorization: Bearer abc ` + + `OPENAI_API_KEY=${unsafeLabels.runtimeValue} ` + + `${unsafeLabels.label}: ${unsafeLabels.pathSegment} ` + + `X-Amz-Signature=${"b".repeat(32)}`, + ), + ); + const networkError = await writeArtifactWorkspace( + { + job: { + _id: "job125", + hasMaliciousSignal: false, + leaseToken: "lease-secret", + source: "publish", + targetKind: "skillVersion", + waitForVtUntil: 0, + }, + target: { + files: [ + { + path: "SKILL.md", + sha256: "ghi789", + size: 60, + url: "https://signed.example.invalid/file?token=secret", + }, + ], + }, + }, + await tempDir(), + ).catch((caught: unknown) => caught); + const networkMessage = + networkError instanceof Error ? networkError.message : String(networkError); + expect(networkError).toBeInstanceOf(Error); + if (!(networkError instanceof Error)) throw new Error("Expected network error"); + const networkCause = networkError.cause; + expect(networkCause).toBeInstanceOf(Error); + const networkCauseMessage = + networkCause instanceof Error ? networkCause.message : String(networkCause); + expect(networkMessage).toContain("Download failed for artifact file SKILL.md"); + expect(networkMessage).not.toContain("https://"); + expect(networkMessage).not.toContain("signed.example.invalid"); + expect(networkMessage).not.toContain("token=secret"); + expect(networkMessage).not.toContain("Authorization"); + expect(networkMessage).not.toContain("Bearer abc"); + expect(networkMessage).not.toContain(" abc"); + expect(networkMessage).not.toContain("OPENAI_API_KEY"); + expect(networkMessage).not.toContain(`${unsafeLabels.label}: ${unsafeLabels.pathSegment}`); + expect(networkMessage).not.toContain(unsafeLabels.pathSegment); + expect(networkMessage).not.toContain(unsafeLabels.runtimeValue); + expect(networkMessage).not.toContain("X-Amz-Signature"); + expect(networkCauseMessage).not.toContain("https://"); + expect(networkCauseMessage).not.toContain("signed.example.invalid"); + expect(networkCauseMessage).not.toContain("token=secret"); + expect(networkCauseMessage).not.toContain("Authorization"); + expect(networkCauseMessage).not.toContain(`${unsafeLabels.label}: ${unsafeLabels.pathSegment}`); + expect(networkCauseMessage).not.toContain(unsafeLabels.pathSegment); + + fetchMock.mockResolvedValueOnce(new Response("forbidden", { status: 403 })); + const clawpackError = await writeArtifactWorkspace( + { + job: { + _id: "job126", + hasMaliciousSignal: false, + leaseToken: "lease-secret", + source: "publish", + targetKind: "packageRelease", + waitForVtUntil: 0, + }, + target: { + clawpackUrl: + "https://signed.example.invalid/package.tgz?token=secret&X-Amz-Signature=abc123", + }, + }, + await tempDir(), + ).catch((caught: unknown) => caught); + const clawpackMessage = + clawpackError instanceof Error ? clawpackError.message : String(clawpackError); + expect(clawpackMessage).toContain("Download failed 403 for artifact tarball artifact.tgz"); + expect(clawpackMessage).not.toContain("https://"); + expect(clawpackMessage).not.toContain("signed.example.invalid"); + expect(clawpackMessage).not.toContain("token=secret"); + expect(clawpackMessage).not.toContain("X-Amz-Signature"); + fetchMock.mockRestore(); + }); + + it("sanitizes download failures before logging or failing the Convex job", async () => { + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response("forbidden", { status: 403 })); + const previousGitHubActions = process.env.GITHUB_ACTIONS; + process.env.GITHUB_ACTIONS = "true"; + const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + const client = { + action: vi.fn(async (..._args: unknown[]) => ({ retry: false })), + }; + + await expect( + processJob( + client, + "worker-token", + { + job: { + _id: "securityScanJobs:download-failed", + hasMaliciousSignal: false, + leaseToken: "lease-secret", + source: "publish", + targetKind: "skillVersion", + waitForVtUntil: 0, + }, + target: { + files: [ + { + path: "SKILL.md", + sha256: "abc123", + size: 42, + url: "https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123", + }, + ], + }, + }, + undefined, + ), + ).resolves.toBe(false); + + expect(client.action).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + error: "Download failed 403 for artifact file SKILL.md", + }), + ); + const logged = stdoutWrite.mock.calls.map((call) => String(call[0])).join("\n"); + expect(logged).toContain( + "::add-mask::https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123", + ); + expect(logged).toContain("security_scan_job_failed"); + expect(logged).toContain("Download failed 403 for artifact file SKILL.md"); + const laterLogs = logged + .split("\n") + .filter((line) => !line.startsWith("::add-mask::")) + .join("\n"); + expect(laterLogs).not.toContain("https://"); + expect(laterLogs).not.toContain("signed.example.invalid"); + expect(laterLogs).not.toContain("token=secret"); + expect(laterLogs).not.toContain("X-Amz-Signature"); + + stdoutWrite.mockRestore(); + if (previousGitHubActions === undefined) delete process.env.GITHUB_ACTIONS; + else process.env.GITHUB_ACTIONS = previousGitHubActions; + fetchMock.mockRestore(); + }); + + it("sanitizes key-value secrets from non-download failures before logging or failing", async () => { + const previousGitHubActions = process.env.GITHUB_ACTIONS; + process.env.GITHUB_ACTIONS = "true"; + const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + const client = { + action: vi.fn(async (..._args: unknown[]) => ({ retry: false })), + }; + + await expect( + processJob( + client, + "worker-token", + { + job: { + _id: "securityScanJobs:path-failed", + hasMaliciousSignal: false, + leaseToken: "lease-secret", + source: "publish", + targetKind: "skillVersion", + waitForVtUntil: 0, + }, + target: { + files: [ + { + path: + "../OPENAI_API_KEY=scan-process-secret " + + "CONVEX_DEPLOY_KEY=convex-process-secret.md", + sha256: "abc123", + size: 42, + url: "data:text/plain,%23%20Skill", + }, + ], + }, + }, + undefined, + ), + ).resolves.toBe(false); + + const failArgs = client.action.mock.calls[0]?.[1] as { error?: unknown } | undefined; + const error = String(failArgs?.error); + expect(error).toBe("Unsafe artifact path: [redacted-path]"); + expect(error).not.toContain("scan-process-secret"); + expect(error).not.toContain("convex-process-secret"); + const logged = stdoutWrite.mock.calls.map((call) => String(call[0])).join("\n"); + expect(logged).toContain("security_scan_job_failed"); + expect(logged).toContain("Unsafe artifact path: [redacted-path]"); + expect(logged).not.toContain("scan-process-secret"); + expect(logged).not.toContain("convex-process-secret"); + + stdoutWrite.mockRestore(); + if (previousGitHubActions === undefined) delete process.env.GITHUB_ACTIONS; + else process.env.GITHUB_ACTIONS = previousGitHubActions; + }); + it("writes redacted Codex diagnostics without copying submitted artifact files or signed URLs", async () => { const diagnosticsRoot = await tempDir(); const artifactWorkspace = await tempDir(); @@ -359,7 +726,7 @@ describe("run-codex-scan-worker diagnostics", () => { '{"verdict":"benign","scan_findings_in_context":[{"ruleId":"x","expected_for_purpose":true,"note":"quoted artifact payload should not persist"}]}', stderr: "workspace read failed https://signed.example.invalid/file?token=secret", stdout: - '{"type":"error","message":"Codex CLI provider returned HTTP 429 for https://signed.example.invalid/file?token=secret with api_key=sk-short-secret"}\n{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"I could not inspect the artifact because the provider returned a transient error."}}\n{"type":"tool_call","status":"failed","api_key":"sk-short-secret","output":"read https://signed.example.invalid/file?token=secret","content":["quoted array artifact payload should not persist"]}\n', + '{"type":"error","message":"Codex CLI provider returned HTTP 429 for https://signed.example.invalid/file?token=secret with api_key=sk-short-fixture"}\n{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"I could not inspect the artifact because the provider returned a transient error."}}\n{"type":"tool_call","status":"failed","source":"artifact controlled source string","api_key":"sk-short-fixture","output":"read https://signed.example.invalid/file?token=secret","content":["quoted array artifact payload should not persist"],"code-snippet":["hyphenated artifact payload should not persist"],"raw_result":["snake artifact payload should not persist"],"userImpact":["camel artifact payload should not persist"],"token":123456,"headers":{"authorization":["Bearer numeric-secret"]}}\n', }, skillSpector: { args: ["scan", "artifact", "--format", "json"], @@ -383,7 +750,7 @@ describe("run-codex-scan-worker diagnostics", () => { target: { files: [ { - path: "SKILL.md", + path: "artifacts/token=artifact-path-secret.md", sha256: "abc123", size: 42, url: "https://signed.example.invalid/file?token=secret", @@ -414,16 +781,28 @@ describe("run-codex-scan-worker diagnostics", () => { const jobDir = join(diagnosticsRoot, "job123"); const stdoutText = await readFile(join(jobDir, "codex.stdout.redacted.jsonl"), "utf8"); expect(stdoutText).toContain('"tool_call"'); - expect(stdoutText).toContain("Codex CLI provider returned HTTP 429"); - expect(stdoutText).toContain( + expect(stdoutText).not.toContain("Codex CLI provider returned HTTP 429"); + expect(stdoutText).not.toContain( "I could not inspect the artifact because the provider returned a transient error.", ); expect(stdoutText).not.toContain("token=secret"); expect(stdoutText).not.toContain("signed.example.invalid"); - expect(stdoutText).not.toContain("sk-short-secret"); + expect(stdoutText).not.toContain("sk-short-fixture"); + expect(stdoutText).not.toContain("123456"); + expect(stdoutText).not.toContain("numeric-secret"); expect(stdoutText).not.toContain("quoted array artifact payload"); + expect(stdoutText).not.toContain("hyphenated artifact payload"); + expect(stdoutText).not.toContain("snake artifact payload"); + expect(stdoutText).not.toContain("camel artifact payload"); expect(stdoutText).toContain('"api_key":"[redacted-secret]"'); + expect(stdoutText).toContain('"token":"[redacted-secret]"'); + expect(stdoutText).toContain('"authorization":"[redacted-secret]"'); + expect(stdoutText).toContain('"source":"[redacted '); + expect(stdoutText).not.toContain("artifact controlled source"); expect(stdoutText).toContain('"content":"[redacted 1 item(s)]"'); + expect(stdoutText).toContain('"code-snippet":"[redacted 1 item(s)]"'); + expect(stdoutText).toContain('"raw_result":"[redacted 1 item(s)]"'); + expect(stdoutText).toContain('"userImpact":"[redacted 1 item(s)]"'); await expect(readFile(join(jobDir, "codex.stderr.redacted.log"), "utf8")).resolves.toContain( "workspace read failed", ); @@ -460,13 +839,23 @@ describe("run-codex-scan-worker diagnostics", () => { expect(diagnostic.error).toBe( "Codex result did not match ClawScan schema: [redacted result body]", ); - expect(diagnostic.target.files).toEqual([{ path: "SKILL.md", sha256: "abc123", size: 42 }]); + expect(diagnostic.target.files).toEqual([ + { path: "[redacted-path]", sha256: "abc123", size: 42 }, + ]); const diagnosticText = await readFile(join(jobDir, "diagnostic.json"), "utf8"); expect(diagnosticText).not.toContain("lease-secret"); + expect(diagnosticText).not.toContain("artifact-path-secret"); expect(diagnosticText).not.toContain("token=secret"); expect(diagnosticText).not.toContain("quoted artifact payload"); expect(diagnosticText).not.toContain("SkillSpector artifact payload"); + const allDiagnosticText = await readAllFilesText(jobDir); + expect(allDiagnosticText).not.toContain("lease-secret"); + expect(allDiagnosticText).not.toContain("token=secret"); + expect(allDiagnosticText).not.toContain("signed.example.invalid"); + expect(allDiagnosticText).not.toContain("sk-short-fixture"); + expect(allDiagnosticText).not.toContain("quoted artifact payload"); + expect(allDiagnosticText).not.toContain("SkillSpector artifact payload"); expect(await readdir(jobDir)).not.toContain("artifact"); }); }); diff --git a/scripts/security/run-codex-scan-worker.ts b/scripts/security/run-codex-scan-worker.ts index b5ba1951..8f77f71a 100644 --- a/scripts/security/run-codex-scan-worker.ts +++ b/scripts/security/run-codex-scan-worker.ts @@ -14,6 +14,14 @@ import { SKILL_SECURITY_EVALUATOR_SYSTEM_PROMPT, } from "../../convex/lib/securityPrompt"; import { assertCodexWorkerExecutionAllowed, resolveCodexWorkerHome } from "../codex-worker-guard"; +import { createWorkerLogger } from "../lib/workerLogger"; +import { + maskGitHubActionsSecret, + maskKnownWorkerSecrets, + redactWorkerPublicErrorMessage, + redactWorkerPublicText, + safeWorkerArtifactPathLabel, +} from "../lib/workerRedaction"; type ClaimedJob = { job: { @@ -99,6 +107,8 @@ type JobDiagnosticInput = { status: "completed" | "failed"; }; +type CodexScanWorkerClient = Pick; + const DEFAULT_BATCH_LIMIT = 4; const DEFAULT_MAX_RUNTIME_MS = 40 * 60 * 1000; const DEFAULT_CODEX_SCAN_TIMEOUT_MS = 20 * 60 * 1000; @@ -107,6 +117,7 @@ const MAX_STORED_SKILLSPECTOR_ISSUES = 25; const MAX_STORED_SKILLSPECTOR_TEXT_CHARS = 2_000; const MAX_STORED_SKILLSPECTOR_SHORT_TEXT_CHARS = 512; const DEFAULT_LEASE_MS = 60 * 60 * 1000; +const logger = createWorkerLogger({ name: "security-scan-worker" }); const root = resolve(new URL("../..", import.meta.url).pathname); const schemaPath = join(root, "scripts/security/codex-scan-output.schema.json"); @@ -183,23 +194,7 @@ function safeDiagnosticPathSegment(value: string) { } function redactDiagnosticText(value: string, maxChars = MAX_DIAGNOSTIC_TEXT_CHARS) { - const redacted = value - .replace(/https?:\/\/[^\s"')<>]+/g, "[redacted-url]") - .replace( - /\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]{12,}/gi, - (_match, scheme: string) => `${scheme} [redacted-secret]`, - ) - .replace( - /\b(token|secret|password|api[_-]?key|authorization)(["']?\s*[:=]\s*["']?)[^\s"',}]+/gi, - (_match, key: string, separator: string) => `${key}${separator}[redacted-secret]`, - ) - .replace( - /\b([A-Z0-9_]*(?:TOKEN|SECRET|PASSWORD|API[_-]?KEY|AUTHORIZATION))(["']?\s*[:=]\s*["']?)[^\s"',}]+/gi, - (_match, key: string, separator: string) => `${key}${separator}[redacted-secret]`, - ) - .replace(/\b[A-Za-z0-9_+/=-]{64,}\b/g, "[redacted-secret]"); - if (redacted.length <= maxChars) return redacted; - return `${redacted.slice(0, maxChars)}\n...[truncated ${redacted.length - maxChars} chars]`; + return redactWorkerPublicText(value, maxChars); } function redactDiagnosticError(value: string) { @@ -209,46 +204,103 @@ function redactDiagnosticError(value: string) { ); } -const DIAGNOSTIC_CONTENT_KEY_PATTERN = - /^(code[_-]?snippet|content|detail|evidence|explanation|finding|findings|guidance|match|message|note|notes|output|rawResult|recommendation|result|snippet|stderr|stdout|summary|text|userImpact|user_impact)$/i; -const DIAGNOSTIC_SECRET_KEY_PATTERN = - /(api[_-]?key|authorization|password|secret|token|webhook|credential)/i; -const CODEX_EVENT_DIAGNOSTIC_TEXT_KEYS = new Set(["message", "text"]); +function sanitizeWorkerErrorMessage(value: string) { + return redactWorkerPublicErrorMessage(redactDiagnosticError(value)); +} -function redactDiagnosticValue(value: unknown, key = "", preserveDiagnosticText = false): unknown { - if (DIAGNOSTIC_SECRET_KEY_PATTERN.test(key)) return "[redacted-secret]"; +const DIAGNOSTIC_CONTENT_TEXT_KEYS = new Set([ + "codesnippet", + "content", + "detail", + "evidence", + "explanation", + "finding", + "findings", + "guidance", + "match", + "message", + "note", + "notes", + "output", + "rawresult", + "recommendation", + "result", + "snippet", + "stderr", + "stdout", + "summary", + "text", + "userimpact", +]); +const DIAGNOSTIC_PUBLIC_TEXT_PATHS = new Set([ + "codexresult.verdict", + "codexstdout.item.id", + "codexstdout.item.type", + "codexstdout.status", + "codexstdout.type", + "llmanalysis.confidence", + "llmanalysis.status", + "llmanalysis.verdict", + "skillspectoranalysis.issues.*.issueid", + "skillspectoranalysis.issues.*.severity", + "skillspectoranalysis.recommendation", + "skillspectoranalysis.scannerversion", + "skillspectoranalysis.severity", + "skillspectoranalysis.status", +]); +const DIAGNOSTIC_PUBLIC_TEXT_VALUE_PATTERN = /^[A-Za-z0-9_.:@/-]{1,160}$/; + +function normalizeDiagnosticKey(key: string) { + return key.replace(/[_-]/g, "").toLowerCase(); +} + +function diagnosticPathKey(path: string[]) { + return path.map((part) => (part === "*" ? part : normalizeDiagnosticKey(part))).join("."); +} + +function isDiagnosticContentTextPath(path: string[]) { + const key = path.at(-1) ?? ""; + return DIAGNOSTIC_CONTENT_TEXT_KEYS.has(normalizeDiagnosticKey(key)); +} + +function isDiagnosticSecretPath(path: string[]) { + const key = normalizeDiagnosticKey(path.at(-1) ?? ""); + return /(apikey|authorization|credential|password|secret|token|webhook)/i.test(key); +} + +function shouldPreserveDiagnosticText(path: string[], original: string, redacted: string) { + return ( + original === redacted && + DIAGNOSTIC_PUBLIC_TEXT_PATHS.has(diagnosticPathKey(path)) && + DIAGNOSTIC_PUBLIC_TEXT_VALUE_PATTERN.test(redacted) + ); +} + +function redactDiagnosticValue(value: unknown, path: string[] = []): unknown { + if (isDiagnosticSecretPath(path)) return "[redacted-secret]"; if (typeof value === "string") { const redacted = redactDiagnosticText(value, 2_000); - if (preserveDiagnosticText && CODEX_EVENT_DIAGNOSTIC_TEXT_KEYS.has(key)) return redacted; - if (!DIAGNOSTIC_CONTENT_KEY_PATTERN.test(key)) return redacted; + if (shouldPreserveDiagnosticText(path, value, redacted)) return redacted; return `[redacted ${redacted.length} chars]`; } if (Array.isArray(value)) { - if (DIAGNOSTIC_CONTENT_KEY_PATTERN.test(key)) return `[redacted ${value.length} item(s)]`; - return value.map((item) => redactDiagnosticValue(item, "", preserveDiagnosticText)); + if (isDiagnosticContentTextPath(path)) return `[redacted ${value.length} item(s)]`; + return value.map((item) => redactDiagnosticValue(item, [...path, "*"])); } if (!value || typeof value !== "object") return value; return Object.fromEntries( Object.entries(value as Record).map(([entryKey, entryValue]) => [ entryKey, - redactDiagnosticValue(entryValue, entryKey, preserveDiagnosticText), + redactDiagnosticValue(entryValue, [...path, entryKey]), ]), ); } -function redactStructuredDiagnosticText( - value: string, - options?: { preserveDiagnosticText?: boolean }, -) { +function redactStructuredDiagnosticText(value: string, rootKey: string) { const trimmed = value.trim(); - const preserveDiagnosticText = options?.preserveDiagnosticText === true; if (!trimmed) return ""; try { - return JSON.stringify( - redactDiagnosticValue(JSON.parse(trimmed), "", preserveDiagnosticText), - null, - 2, - ); + return JSON.stringify(redactDiagnosticValue(JSON.parse(trimmed), [rootKey]), null, 2); } catch { // Codex --json writes JSONL. Redact parseable lines structurally, then fall back to text redaction. const lines = value.split("\n"); @@ -257,9 +309,7 @@ function redactStructuredDiagnosticText( .map((line) => { if (!line.trim()) return line; try { - return JSON.stringify( - redactDiagnosticValue(JSON.parse(line), "", preserveDiagnosticText), - ); + return JSON.stringify(redactDiagnosticValue(JSON.parse(line), [rootKey])); } catch { return redactDiagnosticText(line, 2_000); } @@ -286,7 +336,10 @@ function sanitizedTargetForDiagnostic(target: ClaimedJob["target"]) { version: pickIdentity(target.version, ["_id", "version", "sha256hash"]), package: pickIdentity(target.package, ["_id", "name", "normalizedName"]), release: pickIdentity(target.release, ["_id", "version", "integritySha256"]), - files: target.files?.map(({ url: _url, ...file }) => file), + files: target.files?.map(({ url: _url, ...file }) => ({ + ...file, + path: safeWorkerArtifactPathLabel(file.path), + })), clawpackUrl: Boolean(target.clawpackUrl), trustedOpenClawPlugin: target.trustedOpenClawPlugin, }; @@ -306,11 +359,14 @@ function sanitizedTargetForArtifactContext(target: ClaimedJob["target"]) { }; } -async function writeDiagnosticText(jobDir: string, fileName: string, value: string | undefined) { +async function writeDiagnosticText( + jobDir: string, + fileName: string, + value: string | undefined, + rootKey: string, +) { if (value === undefined) return undefined; - const redacted = redactStructuredDiagnosticText(value, { - preserveDiagnosticText: fileName === "codex.stdout.redacted.jsonl", - }); + const redacted = redactStructuredDiagnosticText(value, rootKey); await writeFile(join(jobDir, fileName), redacted.endsWith("\n") ? redacted : `${redacted}\n`); return fileName; } @@ -324,31 +380,37 @@ export async function writeJobDiagnostic(input: JobDiagnosticInput) { jobDir, "codex.stdout.redacted.jsonl", input.codex?.stdout, + "codexStdout", ); const stderrPath = await writeDiagnosticText( jobDir, "codex.stderr.redacted.log", input.codex?.stderr, + "codexStderr", ); const rawResultPath = await writeDiagnosticText( jobDir, "codex-result.redacted.json", input.codex?.rawResult, + "codexResult", ); const skillSpectorStdoutPath = await writeDiagnosticText( jobDir, "skillspector.stdout.redacted.log", input.skillSpector?.stdout, + "skillSpectorStdout", ); const skillSpectorStderrPath = await writeDiagnosticText( jobDir, "skillspector.stderr.redacted.log", input.skillSpector?.stderr, + "skillSpectorStderr", ); const skillSpectorRawResultPath = await writeDiagnosticText( jobDir, "skillspector-result.redacted.json", input.skillSpector?.rawResult, + "skillSpectorResult", ); const diagnostic = { @@ -363,9 +425,11 @@ export async function writeJobDiagnostic(input: JobDiagnosticInput) { targetKind: input.job.job.targetKind, waitForVtUntil: input.job.job.waitForVtUntil, }, - llmAnalysis: redactDiagnosticValue(input.llmAnalysis), + llmAnalysis: redactDiagnosticValue(input.llmAnalysis, ["llmAnalysis"]), runId: input.runId, - skillSpectorAnalysis: redactDiagnosticValue(input.skillSpectorAnalysis), + skillSpectorAnalysis: redactDiagnosticValue(input.skillSpectorAnalysis, [ + "skillSpectorAnalysis", + ]), startedAt: input.startedAt, status: input.status, target: sanitizedTargetForDiagnostic(input.job.target), @@ -393,14 +457,28 @@ function safeOutputPath(workspace: string, artifactPath: string) { const out = resolve(workspace, "artifact", normalized); const artifactRoot = resolve(workspace, "artifact"); if (!out.startsWith(`${artifactRoot}/`) && out !== artifactRoot) { - throw new Error(`Unsafe artifact path: ${artifactPath}`); + throw new Error(`Unsafe artifact path: ${safeWorkerArtifactPathLabel(artifactPath)}`); } return out; } -async function download(url: string) { - const response = await fetch(url); - if (!response.ok) throw new Error(`Download failed ${response.status}: ${url}`); +function artifactDownloadDescription(kind: "file" | "clawpack", artifactPath: string) { + const safePath = safeWorkerArtifactPathLabel(artifactPath); + return kind === "file" ? `artifact file ${safePath}` : `artifact tarball ${safePath}`; +} + +async function download(url: string, artifact: { kind: "file" | "clawpack"; path: string }) { + maskGitHubActionsSecret(url); + const description = artifactDownloadDescription(artifact.kind, artifact.path); + let response: Response; + try { + response = await fetch(url); + } catch { + throw new Error(`Download failed for ${description}: network error`, { + cause: new Error("network error"), + }); + } + if (!response.ok) throw new Error(`Download failed ${response.status} for ${description}`); return Buffer.from(await response.arrayBuffer()); } @@ -422,12 +500,15 @@ export async function writeArtifactWorkspace(job: ClaimedJob, workspace: string) for (const file of job.target.files ?? []) { const out = safeOutputPath(workspace, file.path); await mkdir(dirname(out), { recursive: true }); - await writeFile(out, await download(file.url)); + await writeFile(out, await download(file.url, { kind: "file", path: file.path })); } if (job.target.clawpackUrl) { const tarballPath = join(workspace, "artifact.tgz"); - await writeFile(tarballPath, await download(job.target.clawpackUrl)); + await writeFile( + tarballPath, + await download(job.target.clawpackUrl, { kind: "clawpack", path: "artifact.tgz" }), + ); const listing = await runCommand("tar", ["-tzf", tarballPath], { cwd: workspace, timeoutMs: 60_000, @@ -1018,12 +1099,12 @@ async function runCodex( return toStoredLlmAnalysis(parsed); } -async function processJob( - client: ConvexHttpClient, +export async function processJob( + client: CodexScanWorkerClient, token: string, job: ClaimedJob, diagnosticsRoot: string | undefined, -) { +): Promise { const workspace = await mkdtemp(join(tmpdir(), `clawhub-codex-scan-${basename(job.job._id)}-`)); const startedAt = Date.now(); const codex: CodexCommandDiagnostic = {}; @@ -1049,18 +1130,39 @@ async function processJob( runId: process.env.GITHUB_RUN_ID, }); status = "completed"; - console.log(`completed ${job.job._id}: ${llmAnalysis.status}`); + logger.info( + { + durationMs: Date.now() - startedAt, + event: "security_scan_job_completed", + jobId: job.job._id, + scannerPhase: "complete", + status: llmAnalysis.status, + targetKind: job.job.targetKind, + }, + "security scan job completed", + ); return true; } catch (error) { - errorMessage = error instanceof Error ? error.message : String(error); + errorMessage = sanitizeWorkerErrorMessage( + error instanceof Error ? error.message : String(error), + ); const failResult = (await client.action(api.securityScan.failCodexScanJob, { token, jobId: job.job._id as Id<"securityScanJobs">, leaseToken: job.job.leaseToken, error: errorMessage, })) as { retry?: boolean } | undefined; - console.error( - `failed ${job.job._id}: ${errorMessage}${failResult?.retry ? " (will retry)" : ""}`, + logger.error( + { + durationMs: Date.now() - startedAt, + event: "security_scan_job_failed", + jobId: job.job._id, + publicReason: errorMessage, + retry: Boolean(failResult?.retry), + scannerPhase: "process", + targetKind: job.job.targetKind, + }, + "security scan job failed", ); return false; } finally { @@ -1081,7 +1183,14 @@ async function processJob( } catch (diagnosticError) { const message = diagnosticError instanceof Error ? diagnosticError.message : String(diagnosticError); - console.error(`failed to write diagnostic for ${job.job._id}: ${message}`); + logger.error( + { + event: "security_scan_diagnostic_write_failed", + jobId: job.job._id, + publicReason: sanitizeWorkerErrorMessage(message), + }, + "security scan diagnostic write failed", + ); } await rm(workspace, { recursive: true, force: true }); } @@ -1092,17 +1201,28 @@ export async function claimCodexScanJobBatch( claimOne: () => Promise, ) { const results = await Promise.allSettled(Array.from({ length: claimLimit }, () => claimOne())); - return results.flatMap((result) => { + let claimFailures = 0; + const jobs = results.flatMap((result) => { if (result.status === "fulfilled") return result.value; + claimFailures += 1; const message = result.reason instanceof Error ? result.reason.message : String(result.reason); - console.error(`failed to claim security scan job: ${message}`); + logger.error( + { + event: "security_scan_claim_failed", + publicReason: sanitizeWorkerErrorMessage(message), + scannerPhase: "claim", + }, + "failed to claim security scan job", + ); return []; }); + return { claimFailures, jobs }; } async function main() { const { batchLimit, maxJobs, maxRuntimeMs, leaseMs, diagnosticsRoot } = parseArgs(); assertCodexWorkerExecutionAllowed(process.env); + maskKnownWorkerSecrets(); const convexUrl = process.env.CONVEX_URL ?? process.env.VITE_CONVEX_URL; if (!convexUrl) throw new Error("CONVEX_URL or VITE_CONVEX_URL is required"); const token = requireEnv("SECURITY_SCAN_WORKER_TOKEN"); @@ -1118,13 +1238,16 @@ async function main() { let totalCompleted = 0; let totalFailed = 0; - console.log(`diagnostics directory: ${diagnosticsRoot}`); + logger.info( + { diagnosticsRoot, event: "security_scan_diagnostics_directory", workerId }, + "security scan diagnostics directory", + ); while (Date.now() < claimDeadline) { const remainingJobs = maxJobs === undefined ? batchLimit : Math.max(0, maxJobs - totalClaimed); if (remainingJobs === 0) break; const claimLimit = Math.min(batchLimit, remainingJobs); - const jobs = await claimCodexScanJobBatch( + const claimBatch = await claimCodexScanJobBatch( claimLimit, async () => (await client.action(api.securityScan.claimCodexScanJobs, { @@ -1134,7 +1257,19 @@ async function main() { leaseMs, })) as ClaimedJob[], ); - console.log(`claimed ${jobs.length} job(s)`); + const { claimFailures, jobs } = claimBatch; + totalFailed += claimFailures; + logger.info( + { + claimed: jobs.length, + claimFailures, + claimLimit, + event: "security_scan_jobs_claimed", + leaseMs, + workerId, + }, + "claimed security scan jobs", + ); if (jobs.length === 0) break; totalClaimed += jobs.length; @@ -1147,10 +1282,16 @@ async function main() { if (jobs.length < claimLimit) break; } - console.log( - `worker summary: claimed=${totalClaimed} completed=${totalCompleted} failed=${totalFailed} elapsedMs=${ - Date.now() - startedAt - }`, + logger.info( + { + elapsedMs: Date.now() - startedAt, + event: "security_scan_worker_summary", + totalClaimed, + totalCompleted, + totalFailed, + workerId, + }, + "security scan worker summary", ); if (totalFailed > 0) { process.exitCode = 1; diff --git a/scripts/security/security-scan-worker-workflow.test.ts b/scripts/security/security-scan-worker-workflow.test.ts new file mode 100644 index 00000000..8fba86d2 --- /dev/null +++ b/scripts/security/security-scan-worker-workflow.test.ts @@ -0,0 +1,79 @@ +/* @vitest-environment node */ +import { readFile } from "node:fs/promises"; +import { describe, expect, it } from "vitest"; +import { parse as parseYaml } from "yaml"; + +type WorkflowStep = { + env?: Record; + id?: string; + if?: string; + name?: string; + run?: string; + uses?: string; + with?: Record; +}; + +function expectSecretStepAllowlist( + steps: WorkflowStep[], + secretName: string, + allowedStepNames: string[], +) { + for (const step of steps) { + const stepName = step.name ?? step.uses ?? ""; + const hasSecret = + Object.hasOwn(step.env ?? {}, secretName) || + JSON.stringify(step).includes(`secrets.${secretName}`); + expect(hasSecret, `${secretName} on ${stepName}`).toBe(allowedStepNames.includes(stepName)); + } +} + +describe("security-scan-codex workflow", () => { + it("scans diagnostics with TruffleHog before uploading artifacts", async () => { + const workflow = parseYaml( + await readFile(".github/workflows/security-scan-codex.yml", "utf8"), + ) as { + jobs: { + "codex-security-scan": { + env?: Record; + steps: WorkflowStep[]; + }; + }; + }; + const steps = workflow.jobs["codex-security-scan"].steps; + const jobEnv = workflow.jobs["codex-security-scan"].env ?? {}; + const scanIndex = steps.findIndex((step) => step.id === "diagnostics_secret_scan"); + const uploadIndex = steps.findIndex((step) => step.uses === "actions/upload-artifact@v7"); + const scanStep = steps[scanIndex]; + const uploadStep = steps[uploadIndex]; + + expect(scanIndex).toBeGreaterThan(-1); + expect(uploadIndex).toBeGreaterThan(-1); + expect(scanIndex).toBeLessThan(uploadIndex); + expect(scanStep?.run).toContain( + "ghcr.io/trufflesecurity/trufflehog:3.95.5@sha256:56c25710275c4b8d74c4f1346a5e7c606fa7ff4afe996f680b288d0fae3fcd9c", + ); + expect(scanStep?.run).toContain("filesystem /scan"); + expect(scanStep?.run).toContain('-v "$PWD/$CODEX_SECURITY_SCAN_DIAGNOSTICS_DIR:/scan:ro"'); + expect(scanStep?.run).toContain("--only-verified"); + expect(scanStep?.run).toContain("--fail"); + expect(scanStep?.run).not.toContain("--debug"); + expect(uploadStep?.if).toBe( + "${{ !cancelled() && steps.diagnostics_secret_scan.outcome == 'success' }}", + ); + expect(uploadStep?.with?.path).toBe("${{ env.CODEX_SECURITY_SCAN_DIAGNOSTICS_DIR }}"); + expect(jobEnv).not.toHaveProperty("OPENAI_API_KEY"); + expect(jobEnv).not.toHaveProperty("SECURITY_SCAN_WORKER_TOKEN"); + expectSecretStepAllowlist(steps, "OPENAI_API_KEY", [ + "Authenticate Codex CLI", + "Run Codex security worker", + ]); + expectSecretStepAllowlist(steps, "SECURITY_SCAN_WORKER_TOKEN", ["Run Codex security worker"]); + expect(scanStep?.env ?? {}).not.toHaveProperty("OPENAI_API_KEY"); + expect(scanStep?.env ?? {}).not.toHaveProperty("SECURITY_SCAN_WORKER_TOKEN"); + expect(steps.find((step) => step.name === "Check configuration")).toBeUndefined(); + expect(steps.find((step) => step.name === "Run Codex security worker")?.env).toEqual({ + OPENAI_API_KEY: "${{ secrets.OPENAI_API_KEY }}", + SECURITY_SCAN_WORKER_TOKEN: "${{ secrets.SECURITY_SCAN_WORKER_TOKEN }}", + }); + }); +}); diff --git a/scripts/skill-cards/run-skill-card-worker.test.ts b/scripts/skill-cards/run-skill-card-worker.test.ts index ced15f12..63656b3e 100644 --- a/scripts/skill-cards/run-skill-card-worker.test.ts +++ b/scripts/skill-cards/run-skill-card-worker.test.ts @@ -2,7 +2,7 @@ import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { assertCodexWorkerExecutionAllowed, isCodexWorkerExecutionAllowed, @@ -17,13 +17,16 @@ import { DEFAULT_MAX_RUNTIME_MS, neutralTemplatePath, prepareNvidiaSkillCardSkill, + processJob, skillCardWorkerId, trustedRendererPath, + writeWorkspace, } from "./run-skill-card-worker"; const tempDirs: string[] = []; afterEach(async () => { + vi.restoreAllMocks(); await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { force: true, recursive: true }))); }); @@ -207,4 +210,165 @@ describe("run-skill-card-worker Codex skill setup", () => { verify_reason: "", }); }); + + it("omits signed artifact URLs from workspace download failure errors", async () => { + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response("forbidden", { status: 403 })); + const workspace = await tempDir(); + + const error = await writeWorkspace( + { + job: { + _id: "skillCardGenerationJobs:download-failed", + leaseToken: "lease-secret", + source: "scan", + }, + target: { + evidence: {}, + files: [ + { + path: "SKILL.md", + sha256: "abc123", + size: 42, + url: "https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123", + }, + ], + skill: { displayName: "Demo Skill", slug: "demo-skill" }, + version: { version: "1.2.3" }, + }, + }, + workspace, + ).catch((caught: unknown) => caught); + + const message = error instanceof Error ? error.message : String(error); + expect(message).toContain("Download failed 403 for artifact file SKILL.md"); + expect(message).not.toContain("https://"); + expect(message).not.toContain("signed.example.invalid"); + expect(message).not.toContain("token=secret"); + expect(message).not.toContain("X-Amz-Signature"); + fetchMock.mockRestore(); + }); + + it("sanitizes download failures before logging or failing the Convex job", async () => { + const previousGitHubActions = process.env.GITHUB_ACTIONS; + process.env.GITHUB_ACTIONS = "true"; + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response("forbidden", { status: 403 })); + const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + const client = { + action: vi.fn(async () => ({ retry: true })), + }; + + await expect( + processJob( + client, + "worker-token", + { + job: { + _id: "skillCardGenerationJobs:download-failed", + attempts: 2, + leaseToken: "lease-secret", + source: "scan", + }, + target: { + evidence: {}, + files: [ + { + path: "SKILL.md", + sha256: "abc123", + size: 42, + url: "https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123", + }, + ], + skill: { displayName: "Demo Skill", slug: "demo-skill" }, + version: { version: "1.2.3" }, + }, + }, + await tempDir(), + ), + ).resolves.toBe(false); + + expect(client.action).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + error: "Download failed 403 for artifact file SKILL.md", + }), + ); + const logged = stdoutWrite.mock.calls.map((call) => String(call[0])).join("\n"); + expect(logged).toContain( + "::add-mask::https://signed.example.invalid/file?token=secret&X-Amz-Signature=abc123", + ); + expect(logged).toContain("skill_card_job_failed"); + expect(logged).toContain("Download failed 403 for artifact file SKILL.md"); + const laterLogs = logged + .split("\n") + .filter((line) => !line.startsWith("::add-mask::")) + .join("\n"); + expect(laterLogs).not.toContain("https://"); + expect(laterLogs).not.toContain("signed.example.invalid"); + expect(laterLogs).not.toContain("token=secret"); + expect(laterLogs).not.toContain("X-Amz-Signature"); + + stdoutWrite.mockRestore(); + if (previousGitHubActions === undefined) delete process.env.GITHUB_ACTIONS; + else process.env.GITHUB_ACTIONS = previousGitHubActions; + fetchMock.mockRestore(); + }); + + it("sanitizes key-value secrets from non-download failures before logging or failing", async () => { + const previousGitHubActions = process.env.GITHUB_ACTIONS; + process.env.GITHUB_ACTIONS = "true"; + const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + const client = { + action: vi.fn(async (..._args: unknown[]) => ({ retry: true })), + }; + + await expect( + processJob( + client, + "worker-token", + { + job: { + _id: "skillCardGenerationJobs:path-failed", + attempts: 2, + leaseToken: "lease-secret", + source: "scan", + }, + target: { + evidence: {}, + files: [ + { + path: + "../OPENAI_API_KEY=skill-card-process-secret " + + "CONVEX_DEPLOY_KEY=convex-process-secret.md", + sha256: "abc123", + size: 42, + url: "data:text/plain,%23%20Skill", + }, + ], + skill: { displayName: "Demo Skill", slug: "demo-skill" }, + version: { version: "1.2.3" }, + }, + }, + await tempDir(), + ), + ).resolves.toBe(false); + + const failArgs = client.action.mock.calls[0]?.[1] as { error?: unknown } | undefined; + const error = String(failArgs?.error); + expect(error).toBe("Unsafe artifact path: [redacted-path]"); + expect(error).not.toContain("skill-card-process-secret"); + expect(error).not.toContain("convex-process-secret"); + const logged = stdoutWrite.mock.calls.map((call) => String(call[0])).join("\n"); + expect(logged).toContain("skill_card_job_failed"); + expect(logged).toContain("Unsafe artifact path: [redacted-path]"); + expect(logged).not.toContain("skill-card-process-secret"); + expect(logged).not.toContain("convex-process-secret"); + + stdoutWrite.mockRestore(); + if (previousGitHubActions === undefined) delete process.env.GITHUB_ACTIONS; + else process.env.GITHUB_ACTIONS = previousGitHubActions; + }); }); diff --git a/scripts/skill-cards/run-skill-card-worker.ts b/scripts/skill-cards/run-skill-card-worker.ts index bc57f677..ffd68774 100644 --- a/scripts/skill-cards/run-skill-card-worker.ts +++ b/scripts/skill-cards/run-skill-card-worker.ts @@ -8,6 +8,13 @@ import { ConvexHttpClient } from "convex/browser"; import { api } from "../../convex/_generated/api"; import type { Id } from "../../convex/_generated/dataModel"; import { assertCodexWorkerExecutionAllowed, resolveCodexWorkerHome } from "../codex-worker-guard"; +import { createWorkerLogger } from "../lib/workerLogger"; +import { + maskGitHubActionsSecret, + maskKnownWorkerSecrets, + redactWorkerPublicErrorMessage, + safeWorkerArtifactPathLabel, +} from "../lib/workerRedaction"; type ClaimedSkillCardJob = { job: { @@ -36,6 +43,7 @@ type CommandResult = { }; type JsonRecord = Record; +type SkillCardWorkerClient = Pick; export const DEFAULT_BATCH_LIMIT = 4; export const DEFAULT_MAX_RUNTIME_MS = 40 * 60 * 1000; @@ -47,6 +55,7 @@ const NVIDIA_SKILL_DIR = "nvidia-skill-card-generator"; const SKILL_CARD_CONTEXT_FILE = "skill-card.context.json"; const SKILL_CARD_OUTPUT_FILE = "skill-card.md"; const LOCAL_CODEX_HOME = join(root, ".codex/runtime/codex-workers/skill-card"); +const logger = createWorkerLogger({ name: "skill-card-worker" }); const NVIDIA_ONLY_PUBLIC_CARD_PATTERNS = [ "NVIDIA believes", "For Release on NVIDIA Platforms Only", @@ -127,14 +136,27 @@ function safeOutputPath(workspace: string, artifactPath: string) { const out = resolve(workspace, "artifact", normalized); const artifactRoot = resolve(workspace, "artifact"); if (!out.startsWith(`${artifactRoot}/`) && out !== artifactRoot) { - throw new Error(`Unsafe artifact path: ${artifactPath}`); + throw new Error(`Unsafe artifact path: ${safeWorkerArtifactPathLabel(artifactPath)}`); } return out; } -async function download(url: string) { - const response = await fetch(url); - if (!response.ok) throw new Error(`Download failed ${response.status}: ${url}`); +function artifactDownloadDescription(artifactPath: string) { + return `artifact file ${safeWorkerArtifactPathLabel(artifactPath)}`; +} + +async function download(url: string, artifact: { path: string }) { + maskGitHubActionsSecret(url); + const description = artifactDownloadDescription(artifact.path); + let response: Response; + try { + response = await fetch(url); + } catch { + throw new Error(`Download failed for ${description}: network error`, { + cause: new Error("network error"), + }); + } + if (!response.ok) throw new Error(`Download failed ${response.status} for ${description}`); return Buffer.from(await response.arrayBuffer()); } @@ -324,7 +346,7 @@ export async function prepareNvidiaSkillCardSkill(workspace: string, toolDir: st return destination; } -async function writeWorkspace(job: ClaimedSkillCardJob, workspace: string) { +export async function writeWorkspace(job: ClaimedSkillCardJob, workspace: string) { await mkdir(join(workspace, "artifact"), { recursive: true }); await writeFile( join(workspace, "evidence.json"), @@ -333,7 +355,7 @@ async function writeWorkspace(job: ClaimedSkillCardJob, workspace: string) { for (const file of job.target.files) { const out = safeOutputPath(workspace, file.path); await mkdir(dirname(out), { recursive: true }); - await writeFile(out, await download(file.url)); + await writeFile(out, await download(file.url, { path: file.path })); } } @@ -413,13 +435,14 @@ export function assertPublicSkillCardMarkdown(markdown: string) { } } -async function processJob( - client: ConvexHttpClient, +export async function processJob( + client: SkillCardWorkerClient, token: string, job: ClaimedSkillCardJob, toolDir: string, ) { const workspace = await mkdtemp(join(tmpdir(), `clawhub-skill-card-${basename(job.job._id)}-`)); + const startedAt = Date.now(); try { await writeWorkspace(job, workspace); await prepareNvidiaSkillCardSkill(workspace, toolDir); @@ -431,17 +454,40 @@ async function processJob( markdown, runId: process.env.GITHUB_RUN_ID, }); - console.log(`completed ${job.job._id}: skill-card.md`); + logger.info( + { + durationMs: Date.now() - startedAt, + event: "skill_card_job_completed", + jobId: job.job._id, + scannerPhase: "complete", + skillSlug: job.target.skill.slug, + }, + "skill card job completed", + ); return true; } catch (error) { - const message = error instanceof Error ? error.message : String(error); + const message = redactWorkerPublicErrorMessage( + error instanceof Error ? error.message : String(error), + ); const failResult = (await client.action(api.skillCards.failSkillCardJob, { token, jobId: job.job._id as Id<"skillCardGenerationJobs">, leaseToken: job.job.leaseToken, error: message, })) as { retry?: boolean } | undefined; - console.error(`failed ${job.job._id}: ${message}${failResult?.retry ? " (will retry)" : ""}`); + logger.error( + { + attempts: job.job.attempts, + durationMs: Date.now() - startedAt, + event: "skill_card_job_failed", + jobId: job.job._id, + publicReason: message, + retry: Boolean(failResult?.retry), + scannerPhase: "process", + skillSlug: job.target.skill.slug, + }, + "skill card job failed", + ); return false; } finally { await rm(workspace, { recursive: true, force: true }); @@ -451,6 +497,7 @@ async function processJob( async function main() { const { batchLimit, maxJobs, maxRuntimeMs, leaseMs, toolDir } = parseArgs(); assertCodexWorkerExecutionAllowed(process.env); + maskKnownWorkerSecrets(); const convexUrl = process.env.CONVEX_URL ?? process.env.VITE_CONVEX_URL; if (!convexUrl) throw new Error("CONVEX_URL or VITE_CONVEX_URL is required"); const token = workerToken(); @@ -466,13 +513,39 @@ async function main() { const remainingJobs = maxJobs === undefined ? batchLimit : Math.max(0, maxJobs - totalClaimed); if (remainingJobs === 0) break; const claimLimit = Math.min(batchLimit, remainingJobs); - const jobs = (await client.action(api.skillCards.claimSkillCardJobs, { - token, - workerId, - limit: claimLimit, - leaseMs, - })) as ClaimedSkillCardJob[]; - console.log(`claimed ${jobs.length} skill card job(s)`); + let jobs: ClaimedSkillCardJob[]; + try { + jobs = (await client.action(api.skillCards.claimSkillCardJobs, { + token, + workerId, + limit: claimLimit, + leaseMs, + })) as ClaimedSkillCardJob[]; + } catch (error) { + logger.error( + { + event: "skill_card_claim_failed", + publicReason: redactWorkerPublicErrorMessage( + error instanceof Error ? error.message : String(error), + ), + scannerPhase: "claim", + workerId, + }, + "failed to claim skill card jobs", + ); + totalFailed += 1; + break; + } + logger.info( + { + claimed: jobs.length, + claimLimit, + event: "skill_card_jobs_claimed", + leaseMs, + workerId, + }, + "claimed skill card jobs", + ); if (jobs.length === 0) break; totalClaimed += jobs.length; @@ -484,10 +557,16 @@ async function main() { if (jobs.length < claimLimit) break; } - console.log( - `skill card worker summary: claimed=${totalClaimed} completed=${totalCompleted} failed=${totalFailed} elapsedMs=${ - Date.now() - startedAt - }`, + logger.info( + { + elapsedMs: Date.now() - startedAt, + event: "skill_card_worker_summary", + totalClaimed, + totalCompleted, + totalFailed, + workerId, + }, + "skill card worker summary", ); if (totalFailed > 0) process.exitCode = 1; } diff --git a/scripts/skill-cards/skill-card-worker-workflow.test.ts b/scripts/skill-cards/skill-card-worker-workflow.test.ts index 0e1ea3f4..d0fc4eb8 100644 --- a/scripts/skill-cards/skill-card-worker-workflow.test.ts +++ b/scripts/skill-cards/skill-card-worker-workflow.test.ts @@ -3,6 +3,28 @@ import { readFile } from "node:fs/promises"; import { describe, expect, it } from "vitest"; import { parse as parseYaml } from "yaml"; +type WorkflowStep = { + env?: Record; + name?: string; + run?: string; + uses?: string; + with?: Record; +}; + +function expectSecretStepAllowlist( + steps: WorkflowStep[], + secretName: string, + allowedStepNames: string[], +) { + for (const step of steps) { + const stepName = step.name ?? step.uses ?? ""; + const hasSecret = + Object.hasOwn(step.env ?? {}, secretName) || + JSON.stringify(step).includes(`secrets.${secretName}`); + expect(hasSecret, `${secretName} on ${stepName}`).toBe(allowedStepNames.includes(stepName)); + } +} + describe("skill-card-worker workflow", () => { it("does not expose OPENAI_API_KEY to the artifact-processing worker step", async () => { const workflow = parseYaml( @@ -13,7 +35,7 @@ describe("skill-card-worker workflow", () => { env?: Record; "timeout-minutes"?: number; strategy?: { matrix?: { shard?: number[] } }; - steps: Array<{ name?: string; env?: Record }>; + steps: WorkflowStep[]; }; }; concurrency?: unknown; @@ -45,11 +67,18 @@ describe("skill-card-worker workflow", () => { "github-actions:${{ github.run_id }}:${{ github.run_attempt }}:${{ matrix.shard }}", ); expect(job.env).not.toHaveProperty("OPENAI_API_KEY"); + expect(job.env).not.toHaveProperty("SECURITY_SCAN_WORKER_TOKEN"); + expectSecretStepAllowlist(job.steps, "OPENAI_API_KEY", ["Authenticate Codex CLI"]); + expectSecretStepAllowlist(job.steps, "SECURITY_SCAN_WORKER_TOKEN", ["Run Skill Card worker"]); + expect(job.steps.find((step) => step.name === "Check configuration")).toBeUndefined(); expect(job.steps.find((step) => step.name === "Authenticate Codex CLI")?.env).toHaveProperty( "OPENAI_API_KEY", ); expect( job.steps.find((step) => step.name === "Run Skill Card worker")?.env ?? {}, ).not.toHaveProperty("OPENAI_API_KEY"); + expect(job.steps.find((step) => step.name === "Run Skill Card worker")?.env).toHaveProperty( + "SECURITY_SCAN_WORKER_TOKEN", + ); }); }); diff --git a/scripts/worker-console-guard.test.ts b/scripts/worker-console-guard.test.ts new file mode 100644 index 00000000..b6dda2ec --- /dev/null +++ b/scripts/worker-console-guard.test.ts @@ -0,0 +1,24 @@ +/* @vitest-environment node */ +import { readFile } from "node:fs/promises"; +import { describe, expect, it } from "vitest"; + +const PRODUCTION_WORKER_SCRIPTS = [ + "scripts/security/run-codex-scan-worker.ts", + "scripts/skill-cards/run-skill-card-worker.ts", +]; + +describe("production worker console guard", () => { + it("keeps raw console logging out of production worker scripts", async () => { + const violations: string[] = []; + for (const path of PRODUCTION_WORKER_SCRIPTS) { + const text = await readFile(path, "utf8"); + for (const [index, line] of text.split(/\r?\n/).entries()) { + if (/\bconsole\.(?:log|warn|error)\s*\(/.test(line)) { + violations.push(`${path}:${index + 1}: ${line.trim()}`); + } + } + } + + expect(violations).toEqual([]); + }); +});