From 0a6b9c291ba768c643245d51d48a7d79bf08d86a Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Mon, 3 Aug 2026 11:27:41 +0200 Subject: [PATCH] Prevent npm lookup diagnostics from leaking credentials or paths The merged release preflight fix left two review threads open. Its sanitizer now removes complete Bearer values and supported Unix and Windows absolute paths, while the new preflight cases use the repository BDD test API. Constraint: Registry failures are written to GitHub Actions logs and must not expose credentials or machine-local paths. Rejected: Redact only token-safe URL characters | Bearer credentials may contain slash, plus, and padding characters. Confidence: high Scope-risk: narrow Tested: publish preflight 4 tests and 2 BDD steps; fmt; lint; typecheck; bash syntax; git diff check --- scripts/ci/publish-npm-packages.sh | 7 +- scripts/ci/publish-npm-packages.test.ts | 219 +++++++++++++----------- scripts/deno.lock | 4 + 3 files changed, 122 insertions(+), 108 deletions(-) diff --git a/scripts/ci/publish-npm-packages.sh b/scripts/ci/publish-npm-packages.sh index c224f0b830..7259d83a07 100755 --- a/scripts/ci/publish-npm-packages.sh +++ b/scripts/ci/publish-npm-packages.sh @@ -168,12 +168,11 @@ sanitize_npm_lookup_output() { printf '%s\n' "$1" \ | sed -E \ -e '/^npm error A complete log of this run can be found in:/d' \ - -e 's#Bearer [A-Za-z0-9._~-]+#Bearer #g' \ + -e 's#Bearer [^[:space:]]+#Bearer #g' \ -e 's#([?&]token=)[^[:space:]&]+#\1#g' \ -e 's#(_authToken=)[^[:space:]]+#\1#g' \ - -e 's#/Users/[^[:space:]]+##g' \ - -e 's#/home/[^[:space:]]+##g' \ - -e 's#/var/folders/[^[:space:]]+##g' + -e 's#(^|[[:space:]=(])/(Users|home|var|tmp|private/tmp)/[^[:space:]]+#\1#g' \ + -e 's#(^|[[:space:]=(])[A-Za-z]:[\\/][^[:space:]]+#\1#g' } ensure_package_names_registered() { diff --git a/scripts/ci/publish-npm-packages.test.ts b/scripts/ci/publish-npm-packages.test.ts index 5d8749d3ef..42db1cdf3f 100644 --- a/scripts/ci/publish-npm-packages.test.ts +++ b/scripts/ci/publish-npm-packages.test.ts @@ -1,4 +1,5 @@ import { assertEquals, assertStringIncludes } from "#std/assert"; +import { describe, it } from "#veryfront/testing/bdd.ts"; const scriptPath = `${Deno.cwd()}/scripts/ci/publish-npm-packages.sh`; const decoder = new TextDecoder(); @@ -142,113 +143,123 @@ Deno.test("npm release rerun skips a package already published for the commit", } }); -Deno.test("npm release preflight rejects an E404 unbootstrapped package name", async () => { - const stateDir = await Deno.makeTempDir(); - const npmLog = `${stateDir}/npm.log`; - await Deno.writeTextFile(npmLog, ""); +describe("npm release preflight", () => { + it("rejects an E404 unbootstrapped package name", async () => { + const stateDir = await Deno.makeTempDir(); + const npmLog = `${stateDir}/npm.log`; + await Deno.writeTextFile(npmLog, ""); - try { - const output = await runBash( - [ - "set -euo pipefail", - 'source "$SCRIPT_PATH"', - "package_names_from_workspace() {", - ' printf "%s\\n" "@veryfront/ext-existing" "@veryfront/ext-new"', - "}", - "npm() {", - ' printf "%s\\n" "$*" >> "$NPM_LOG"', - ' if [ "$1" = "view" ] && [ "$2" = "@veryfront/ext-existing@*" ] && [ "$3" = "name" ]; then', - ' printf "%s\\n" "@veryfront/ext-existing"', - " return 0", - " fi", - ' printf "%s\\n" "npm error code E404" >&2', - ' printf "%s\\n" "npm error 404 Not Found - GET https://registry.npmjs.org/@veryfront%2fext-new - Not found" >&2', - " return 1", - "}", - "run_preflight", - ].join("\n"), - { - GITHUB_SHA: "expected-commit", - NPM_LOG: npmLog, - VERSION: "0.1.1189", - }, - ); + try { + const output = await runBash( + [ + "set -euo pipefail", + 'source "$SCRIPT_PATH"', + "package_names_from_workspace() {", + ' printf "%s\\n" "@veryfront/ext-existing" "@veryfront/ext-new"', + "}", + "npm() {", + ' printf "%s\\n" "$*" >> "$NPM_LOG"', + ' if [ "$1" = "view" ] && [ "$2" = "@veryfront/ext-existing@*" ] && [ "$3" = "name" ]; then', + ' printf "%s\\n" "@veryfront/ext-existing"', + " return 0", + " fi", + ' printf "%s\\n" "npm error code E404" >&2', + ' printf "%s\\n" "npm error 404 Not Found - GET https://registry.npmjs.org/@veryfront%2fext-new - Not found" >&2', + " return 1", + "}", + "run_preflight", + ].join("\n"), + { + GITHUB_SHA: "expected-commit", + NPM_LOG: npmLog, + VERSION: "0.1.1189", + }, + ); - assertEquals(output.code, 1, decoder.decode(output.stderr)); - assertStringIncludes( - decoder.decode(output.stderr), - "@veryfront/ext-new is not registered on npm", - ); - assertStringIncludes( - decoder.decode(output.stderr), - "Publish each package once with a prerelease version and a non-latest dist-tag", - ); - assertEquals( - decoder.decode(output.stderr).includes("npm registry lookup failed"), - false, - ); - const calls = (await Deno.readTextFile(npmLog)).trim().split("\n"); - assertEquals(calls, [ - "view @veryfront/ext-existing@* name", - "view @veryfront/ext-new@* name", - ]); - } finally { - await Deno.remove(stateDir, { recursive: true }); - } -}); + assertEquals(output.code, 1, decoder.decode(output.stderr)); + assertStringIncludes( + decoder.decode(output.stderr), + "@veryfront/ext-new is not registered on npm", + ); + assertStringIncludes( + decoder.decode(output.stderr), + "Publish each package once with a prerelease version and a non-latest dist-tag", + ); + assertEquals( + decoder.decode(output.stderr).includes("npm registry lookup failed"), + false, + ); + const calls = (await Deno.readTextFile(npmLog)).trim().split("\n"); + assertEquals(calls, [ + "view @veryfront/ext-existing@* name", + "view @veryfront/ext-new@* name", + ]); + } finally { + await Deno.remove(stateDir, { recursive: true }); + } + }); -Deno.test("npm release preflight reports non-E404 registry lookup failures", async () => { - const stateDir = await Deno.makeTempDir(); - const npmLog = `${stateDir}/npm.log`; - await Deno.writeTextFile(npmLog, ""); + it("reports sanitized non-E404 registry lookup failures", async () => { + const stateDir = await Deno.makeTempDir(); + const npmLog = `${stateDir}/npm.log`; + await Deno.writeTextFile(npmLog, ""); - try { - const output = await runBash( - [ - "set -euo pipefail", - 'source "$SCRIPT_PATH"', - "package_names_from_workspace() {", - ' printf "%s\\n" "@veryfront/ext-existing" "@veryfront/ext-flaky"', - "}", - "npm() {", - ' printf "%s\\n" "$*" >> "$NPM_LOG"', - ' if [ "$1" = "view" ] && [ "$2" = "@veryfront/ext-existing@*" ] && [ "$3" = "name" ]; then', - ' printf "%s\\n" "@veryfront/ext-existing"', - " return 0", - " fi", - ' printf "%s\\n" "npm error code E503" >&2', - ' printf "%s\\n" "npm error 503 Service Unavailable" >&2', - ' printf "%s\\n" "npm error A complete log of this run can be found in: /Users//.npm/_logs/debug.log" >&2', - " return 42", - "}", - "run_preflight", - ].join("\n"), - { - GITHUB_SHA: "expected-commit", - NPM_LOG: npmLog, - VERSION: "0.1.1189", - }, - ); + try { + const output = await runBash( + [ + "set -euo pipefail", + 'source "$SCRIPT_PATH"', + "package_names_from_workspace() {", + ' printf "%s\\n" "@veryfront/ext-existing" "@veryfront/ext-flaky"', + "}", + "npm() {", + ' printf "%s\\n" "$*" >> "$NPM_LOG"', + ' if [ "$1" = "view" ] && [ "$2" = "@veryfront/ext-existing@*" ] && [ "$3" = "name" ]; then', + ' printf "%s\\n" "@veryfront/ext-existing"', + " return 0", + " fi", + ' printf "%s\\n" "npm error code E503" >&2', + ' printf "%s\\n" "npm error 503 Service Unavailable" >&2', + ' printf "%s\\n" "npm error A complete log of this run can be found in: /Users//.npm/_logs/debug.log" >&2', + ' printf "%s\\n" "npm error authorization Bearer token-part/with=suffix" >&2', + ' printf "%s\\n" "npm error cache path=/tmp/npm-cache/debug.log" >&2', + ' printf "%s\\n" "npm error cache path=C:\\\\runner\\\\npm-cache\\\\debug.log" >&2', + " return 42", + "}", + "run_preflight", + ].join("\n"), + { + GITHUB_SHA: "expected-commit", + NPM_LOG: npmLog, + VERSION: "0.1.1189", + }, + ); - assertEquals(output.code, 1, decoder.decode(output.stderr)); - const stderr = decoder.decode(output.stderr); - assertStringIncludes( - stderr, - "npm registry lookup failed for @veryfront/ext-flaky (status 42)", - ); - assertStringIncludes(stderr, "npm error code E503"); - assertEquals(stderr.includes("is not registered on npm"), false); - assertEquals( - stderr.includes("Publish each package once with a prerelease version"), - false, - ); - assertEquals(stderr.includes("/Users/"), false); - const calls = (await Deno.readTextFile(npmLog)).trim().split("\n"); - assertEquals(calls, [ - "view @veryfront/ext-existing@* name", - "view @veryfront/ext-flaky@* name", - ]); - } finally { - await Deno.remove(stateDir, { recursive: true }); - } + assertEquals(output.code, 1, decoder.decode(output.stderr)); + const stderr = decoder.decode(output.stderr); + assertStringIncludes( + stderr, + "npm registry lookup failed for @veryfront/ext-flaky (status 42)", + ); + assertStringIncludes(stderr, "npm error code E503"); + assertEquals(stderr.includes("is not registered on npm"), false); + assertEquals( + stderr.includes("Publish each package once with a prerelease version"), + false, + ); + assertEquals(stderr.includes("/Users/"), false); + assertEquals(stderr.includes("token-part"), false); + assertEquals(stderr.includes("with=suffix"), false); + assertEquals(stderr.includes("/tmp/"), false); + assertEquals(stderr.includes("C:\\runner"), false); + assertStringIncludes(stderr, "Bearer "); + const calls = (await Deno.readTextFile(npmLog)).trim().split("\n"); + assertEquals(calls, [ + "view @veryfront/ext-existing@* name", + "view @veryfront/ext-flaky@* name", + ]); + } finally { + await Deno.remove(stateDir, { recursive: true }); + } + }); }); diff --git a/scripts/deno.lock b/scripts/deno.lock index 4111744f8d..7f18e98b97 100644 --- a/scripts/deno.lock +++ b/scripts/deno.lock @@ -18,6 +18,7 @@ "jsr:@ts-morph/common@0.27": "0.27.0", "npm:@babel/parser@7.29.2": "7.29.2", "npm:@mdx-js/mdx@3.1.1": "3.1.1", + "npm:es-module-lexer@2.3.1": "2.3.1", "npm:esbuild@0.28.1": "0.28.1" }, "jsr": { @@ -365,6 +366,9 @@ "dequal" ] }, + "es-module-lexer@2.3.1": { + "integrity": "sha512-shc1dbU90Yl/xq1QrC7QRtfcwURZuVRfPhZbDoldJ1cn1gzDvBaBWlv0eFolj5+0znnPJz5TXLxsN77X/12KTA==" + }, "esast-util-from-estree@2.0.0": { "integrity": "sha512-4CyanoAudUSBAn5K13H4JhsMH6L9ZP7XbLVe/dKybkxMO7eDyLsT8UHl9TRNrU2Gr9nz+FovfSIjuXWJ81uVwQ==", "dependencies": [