Skip to content

Commit b776fb5

Browse files
committed
refactor: fold npmrc-detect into auth module
Review-driven simplification: - Merge propagateProjectNpmrcAuth into src/auth.ts since both the registry-url write path and the .npmrc read path are concerned with the same thing: .npmrc + auth env export - Replace existsSync + readFileSync TOCTOU with try/catch on ENOENT - Dedupe referenced env vars with a single Set instead of Set + array - Drop detectProjectNpmrc / ProjectNpmrc from the public surface; test only the observable behavior of propagateProjectNpmrcAuth - Remove narrative comment in index.ts — the function name carries it
1 parent e79fc32 commit b776fb5

6 files changed

Lines changed: 151 additions & 231 deletions

File tree

dist/index.mjs

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

src/auth.test.ts

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { describe, it, expect, beforeEach, afterEach, vi } from "vite-plus/test";
2-
import { join } from "node:path";
32
import { existsSync, readFileSync, writeFileSync } from "node:fs";
4-
import { configAuthentication } from "./auth.js";
5-
import { exportVariable } from "@actions/core";
3+
import { join } from "node:path";
4+
import { configAuthentication, propagateProjectNpmrcAuth } from "./auth.js";
5+
import { exportVariable, info } from "@actions/core";
66

77
vi.mock("@actions/core", () => ({
88
debug: vi.fn(),
9+
info: vi.fn(),
910
exportVariable: vi.fn(),
1011
}));
1112

@@ -177,3 +178,91 @@ describe("configAuthentication", () => {
177178
expect(exportVariable).toHaveBeenCalledWith("NODE_AUTH_TOKEN", "my-real-token");
178179
});
179180
});
181+
182+
describe("propagateProjectNpmrcAuth", () => {
183+
const projectDir = "/workspace/project";
184+
const npmrcPath = join(projectDir, ".npmrc");
185+
186+
function mockNpmrc(content: string): void {
187+
vi.mocked(readFileSync).mockImplementation((p) => {
188+
if (p === npmrcPath) return content;
189+
const err = Object.assign(new Error("ENOENT"), { code: "ENOENT" });
190+
throw err;
191+
});
192+
}
193+
194+
function mockNoNpmrc(): void {
195+
vi.mocked(readFileSync).mockImplementation(() => {
196+
const err = Object.assign(new Error("ENOENT"), { code: "ENOENT" });
197+
throw err;
198+
});
199+
}
200+
201+
afterEach(() => {
202+
vi.unstubAllEnvs();
203+
vi.resetAllMocks();
204+
});
205+
206+
it("does nothing when there is no project .npmrc", () => {
207+
mockNoNpmrc();
208+
209+
propagateProjectNpmrcAuth(projectDir);
210+
211+
expect(exportVariable).not.toHaveBeenCalled();
212+
});
213+
214+
it("exports referenced env vars that are set in the environment", () => {
215+
mockNpmrc("//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}");
216+
vi.stubEnv("NODE_AUTH_TOKEN", "my-real-token");
217+
218+
propagateProjectNpmrcAuth(projectDir);
219+
220+
expect(exportVariable).toHaveBeenCalledWith("NODE_AUTH_TOKEN", "my-real-token");
221+
expect(info).toHaveBeenCalledWith(expect.stringContaining(".npmrc"));
222+
});
223+
224+
it("skips env vars that are not set", () => {
225+
mockNpmrc("//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}");
226+
vi.stubEnv("NODE_AUTH_TOKEN", "");
227+
228+
propagateProjectNpmrcAuth(projectDir);
229+
230+
expect(exportVariable).not.toHaveBeenCalled();
231+
});
232+
233+
it("does not re-export PATH or HOME even if referenced", () => {
234+
mockNpmrc("cache=${HOME}/.npm-cache");
235+
vi.stubEnv("HOME", "/home/runner");
236+
237+
propagateProjectNpmrcAuth(projectDir);
238+
239+
expect(exportVariable).not.toHaveBeenCalledWith("HOME", expect.anything());
240+
});
241+
242+
it("exports all referenced auth-like env vars, deduping repeats", () => {
243+
mockNpmrc(
244+
[
245+
"//npm.pkg.github.com/:_authToken=${GITHUB_TOKEN}",
246+
"//registry.example.com/:_authToken=${NPM_TOKEN}",
247+
"//other.example.com/:_authToken=${GITHUB_TOKEN}",
248+
].join("\n"),
249+
);
250+
vi.stubEnv("GITHUB_TOKEN", "gh-token");
251+
vi.stubEnv("NPM_TOKEN", "npm-token");
252+
253+
propagateProjectNpmrcAuth(projectDir);
254+
255+
expect(exportVariable).toHaveBeenCalledWith("GITHUB_TOKEN", "gh-token");
256+
expect(exportVariable).toHaveBeenCalledWith("NPM_TOKEN", "npm-token");
257+
const ghCalls = vi.mocked(exportVariable).mock.calls.filter((c) => c[0] === "GITHUB_TOKEN");
258+
expect(ghCalls).toHaveLength(1);
259+
});
260+
261+
it("rethrows non-ENOENT read errors", () => {
262+
vi.mocked(readFileSync).mockImplementation(() => {
263+
throw Object.assign(new Error("EACCES"), { code: "EACCES" });
264+
});
265+
266+
expect(() => propagateProjectNpmrcAuth(projectDir)).toThrow("EACCES");
267+
});
268+
});

src/auth.ts

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { existsSync, readFileSync, writeFileSync } from "node:fs";
22
import { EOL } from "node:os";
3-
import { resolve } from "node:path";
4-
import { debug, exportVariable } from "@actions/core";
3+
import { join, resolve } from "node:path";
4+
import { debug, exportVariable, info } from "@actions/core";
55

66
/**
77
* Configure npm registry authentication by writing a .npmrc file.
@@ -65,3 +65,58 @@ function writeRegistryToFile(registryUrl: string, fileLocation: string, scope?:
6565
// Export placeholder if NODE_AUTH_TOKEN is not set so npm doesn't error
6666
exportVariable("NODE_AUTH_TOKEN", process.env.NODE_AUTH_TOKEN || "XXXXX-XXXXX-XXXXX-XXXXX");
6767
}
68+
69+
// Env vars the runner/system manages — never re-export via GITHUB_ENV
70+
const RESERVED_ENV_VARS = new Set([
71+
"PATH",
72+
"HOME",
73+
"USERPROFILE",
74+
"TMPDIR",
75+
"RUNNER_TEMP",
76+
"RUNNER_OS",
77+
"RUNNER_ARCH",
78+
"GITHUB_ACTIONS",
79+
"GITHUB_WORKSPACE",
80+
"GITHUB_REPOSITORY",
81+
"GITHUB_REPOSITORY_OWNER",
82+
"CI",
83+
]);
84+
85+
/**
86+
* When the project has an `.npmrc` referencing env vars (commonly
87+
* `${NODE_AUTH_TOKEN}` for private registries), re-export the ones that are
88+
* already set so they persist via `GITHUB_ENV` and remain visible to the
89+
* package-manager subprocess spawned by `vp install` and to subsequent steps.
90+
* Lets users rely on their existing `.npmrc` without also passing `registry-url`.
91+
*/
92+
export function propagateProjectNpmrcAuth(projectDir: string): void {
93+
const npmrcPath = join(projectDir, ".npmrc");
94+
let content: string;
95+
try {
96+
content = readFileSync(npmrcPath, "utf8");
97+
} catch (err) {
98+
if ((err as NodeJS.ErrnoException).code === "ENOENT") return;
99+
throw err;
100+
}
101+
102+
const referenced = new Set<string>();
103+
for (const match of content.matchAll(/\$\{(\w+)\}/g)) {
104+
referenced.add(match[1]!);
105+
}
106+
107+
const propagatable = [...referenced].filter(
108+
(name) => !RESERVED_ENV_VARS.has(name) && !!process.env[name],
109+
);
110+
111+
if (propagatable.length === 0) {
112+
debug(`Project .npmrc at ${npmrcPath}: no auth env vars to propagate`);
113+
return;
114+
}
115+
116+
info(
117+
`Detected project .npmrc at ${npmrcPath}. Propagating auth env vars: ${propagatable.join(", ")}`,
118+
);
119+
for (const name of propagatable) {
120+
exportVariable(name, process.env[name]!);
121+
}
122+
}

src/index.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ import { saveCache } from "./cache-save.js";
88
import { State, Outputs } from "./types.js";
99
import type { Inputs } from "./types.js";
1010
import { resolveNodeVersionFile } from "./node-version-file.js";
11-
import { configAuthentication } from "./auth.js";
12-
import { propagateProjectNpmrcAuth } from "./npmrc-detect.js";
11+
import { configAuthentication, propagateProjectNpmrcAuth } from "./auth.js";
1312
import { getConfiguredProjectDir } from "./utils.js";
1413

1514
async function runMain(inputs: Inputs): Promise<void> {
@@ -36,9 +35,6 @@ async function runMain(inputs: Inputs): Promise<void> {
3635
if (inputs.registryUrl) {
3736
configAuthentication(inputs.registryUrl, inputs.scope);
3837
} else {
39-
// No explicit registry-url: respect the project's .npmrc if present.
40-
// Propagate referenced auth env vars (e.g. NODE_AUTH_TOKEN) via GITHUB_ENV
41-
// so they survive into package-manager subprocesses and later steps.
4238
propagateProjectNpmrcAuth(projectDir);
4339
}
4440

src/npmrc-detect.test.ts

Lines changed: 0 additions & 143 deletions
This file was deleted.

0 commit comments

Comments
 (0)