Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/zpm-config/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@
"description": "The URL to use for downloading Node.js distributions",
"default": "https://nodejs.org/dist"
},
"nodeDistAuthHeader": {
"type": ["zpm_utils::Secret<String>", "null"],
"description": "The Authorization header to send when downloading Node.js distributions"
},
"nodeLinker": {
"type": "crate::NodeLinker",
"description": "The linker to use for node_modules",
Expand Down
12 changes: 10 additions & 2 deletions packages/zpm/src/builtins/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@ pub async fn resolve_nodejs_version(context: &InstallContext<'_>, range: &zpm_se
let release_url
= format!("{}/index.json", project.config.settings.node_dist_url.value);

let text
= project.http_client.get(&release_url)?.send_text().await?;
let node_dist_auth_header = project.config.settings.node_dist_auth_header.value.as_ref()
.map(|header| header.value.as_str());

let text = project.http_client.get(&release_url)?
.header("authorization", node_dist_auth_header)
.send_text().await?;

#[derive(Deserialize)]
struct NodejsManifest {
Expand Down Expand Up @@ -154,6 +158,9 @@ pub async fn fetch_nodejs_locator<'a>(context: &InstallContext<'a>, locator: &Lo
let url
= format!("{}/v{}/node-v{}-{}.tar.gz", project.config.settings.node_dist_url.value, version_str, version_str, file_name);

let node_dist_auth_header = project.config.settings.node_dist_auth_header.value.as_ref()
.map(|header| header.value.as_str());

let package_cache = context.package_cache
.expect("The package cache is required for fetching npm packages");
let cache_packer
Expand All @@ -175,6 +182,7 @@ pub async fn fetch_nodejs_locator<'a>(context: &InstallContext<'a>, locator: &Lo
let cached_blob = package_cache.ensure_blob(locator.clone(), ".zip", || async move {
let (_, bytes)
= project.http_client.get(&url)?
.header("authorization", node_dist_auth_header)
.send_bytes().await?;

let archive = tokio::task::spawn_blocking(move || -> Result<Vec<u8>, Error> {
Expand Down
29 changes: 25 additions & 4 deletions tests/acceptance-tests/pkg-tests-core/sources/utils/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,11 @@ export type Request = {
localName: string;
} | {
type: RequestType.NodeDistIndex;
private: boolean;
} | {
type: RequestType.NodeDistTarball;
name: string;
private: boolean;
} | {
type: RequestType.OtelTraces;
body?: unknown;
Expand Down Expand Up @@ -532,6 +534,8 @@ export const validLogins = {
otpUserWithNotice: new Login(`otp-user-with-notice`, {otp: true, notice: true}),
} as const;

export const validNodeDistAuthHeader = `secret-token`;

let whitelist = new Map();
let recording: Array<Request> | null = null;

Expand Down Expand Up @@ -1320,14 +1324,16 @@ exit 0
return {
type: RequestType.Repository,
};
} else if ((match = url.match(/^\/node\/dist\/index.json$/))) {
} else if ((match = url.match(/^\/node(-private)?\/dist\/index.json$/))) {
return {
type: RequestType.NodeDistIndex,
private: typeof match[1] !== `undefined`,
};
} else if ((match = url.match(/^\/node\/dist\/v([0-9]+\.[0-9]+\.[0-9]+)\/(node-v(\1)-[a-z0-9-]+)\.tar\.gz$/))) {
} else if ((match = url.match(/^\/node(-private)?\/dist\/v([0-9]+\.[0-9]+\.[0-9]+)\/(node-v(\2)-[a-z0-9-]+)\.tar\.gz$/))) {
return {
type: RequestType.NodeDistTarball,
name: match[2]!,
name: match[3]!,
private: typeof match[1] !== `undefined`,
};
} else if (url === `/v1/traces`) {
return {
Expand Down Expand Up @@ -1506,7 +1512,22 @@ exit 0
recording.push(parsedRequest);

const {authorization} = req.headers;
if (authorization != null) {
const isPrivateNodeDistRequest = (
parsedRequest.type === RequestType.NodeDistIndex
|| parsedRequest.type === RequestType.NodeDistTarball
) && parsedRequest.private;

if (isPrivateNodeDistRequest) {
if (authorization == null) {
sendError(res, 401, `Authentication required`);
return;
}

if (authorization !== validNodeDistAuthHeader) {
sendError(res, 401, `Invalid token`);
return;
}
} else if (authorization != null) {
const user = validAuthorizations.get(authorization);
if (!user) {
sendError(res, 401, `Invalid token`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,22 +74,30 @@ const options = {
describe(`Commands`, () => {
describe(`config`, () => {
test(`should redact secrets by default`, makeTemporaryEnv({}, async ({path, run}) => {
await xfs.writeFilePromise(ppath.join(path, RC_FILENAME), `npmAuthToken: super-secret-token\n`);
await xfs.writeFilePromise(ppath.join(path, RC_FILENAME), [
`npmAuthToken: super-secret-token`,
`nodeDistAuthHeader: Bearer super-secret-header`,
].join(`\n`));

const {stdout} = await run(`config`, `--json`);
expect(stdout).toContain(`<redacted>`);
expect(stdout).not.toContain(`super-secret-token`);
expect(stdout).not.toContain(`super-secret-header`);
}));

test(`should reveal secrets when --no-redacted is passed`, makeTemporaryEnv({}, async ({path, run}) => {
// Regression for the inverted `--no-redacted` polarity:
// previously the flag was a silent no-op and the token stayed
// `<redacted>` even when the user explicitly opted out of
// redaction.
await xfs.writeFilePromise(ppath.join(path, RC_FILENAME), `npmAuthToken: super-secret-token\n`);
await xfs.writeFilePromise(ppath.join(path, RC_FILENAME), [
`npmAuthToken: super-secret-token`,
`nodeDistAuthHeader: Bearer super-secret-header`,
].join(`\n`));

const {stdout} = await run(`config`, `--no-redacted`, `--json`);
expect(stdout).toContain(`super-secret-token`);
expect(stdout).toContain(`super-secret-header`);
expect(stdout).not.toContain(`<redacted>`);
}));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {Filename, ppath, PortablePath, xfs} from '@yarnpkg/fslib';
import {yarn} from 'pkg-tests-core';
import {tests, yarn} from 'pkg-tests-core';

const {startPackageServer, validNodeDistAuthHeader} = tests;

describe(`Features`, () => {
describe(`Node.js Versioning`, () => {
Expand Down Expand Up @@ -63,6 +65,44 @@ describe(`Features`, () => {
}),
);

describe(`Distribution authentication`, () => {
test(
`it should send the configured authorization header`,
makeTemporaryEnv({
dependencies: {
[`@yarnpkg/node`]: `builtin:^22.0.0`,
},
}, async ({run}) => {
await run(`install`, {
nodeDistUrl: `${await startPackageServer()}/node-private/dist`,
nodeDistAuthHeader: validNodeDistAuthHeader,
env: {
YARN_CPU_OVERRIDE: `x64`,
YARN_OS_OVERRIDE: `linux`,
},
});
}),
);

test(
`it should fail with a descriptive error when the authorization header is invalid`,
makeTemporaryEnv({
dependencies: {
[`@yarnpkg/node`]: `builtin:^22.0.0`,
},
}, async ({run}) => {
await expect(run(`install`, {
nodeDistUrl: `${await startPackageServer()}/node-private/dist`,
nodeDistAuthHeader: `Bearer invalid-node-dist-token`,
env: {
YARN_CPU_OVERRIDE: `x64`,
YARN_OS_OVERRIDE: `linux`,
},
})).rejects.toThrow(/Network error: HTTP status client error \(401 Unauthorized\) for url .*\/node-private\/dist\/index\.json/);
}),
);
});

describe(`Monorepo support`, () => {
test(
`it should allow declaring @yarnpkg/node in a workspace profile`,
Expand Down
20 changes: 20 additions & 0 deletions website/config/yarnrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,26 @@
}
]
},
"nodeDistAuthHeader": {
"_package": "@yarnpkg/core",
"title": "Authorization header to send when downloading Node.js distributions.",
"description": "When specified, the value will be sent as an `Authorization` header when fetching both the release index and distribution archives from `nodeDistUrl`. This is typically paired with a custom `nodeDistUrl` for an internal node.js distribution mirror.",
"type": ["string", "null"],
"_examples": [
{
"description": "Do not pass an `Authorization` header.",
"value": null
},
{
"description": "Pass a Bearer token in the `Authorization` header.",
"value": "Bearer a-token-here"
},
{
"description": "Pass a Bearer token from an environment variable.",
"value": "Bearer ${NODE_DIST_BEARER_TOKEN}"
}
]
},
Comment on lines +1211 to +1230

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also considered a scoped structure similar to npmRegistries -> npmAuthToken :

// .yarnrc.yml
nodeDistAuth:
  "https://mirror-one.example.com/node/dist":
    authorization: "Bearer a-token"

  "https://mirror-two.example.com/node/dist":
    authorization: "Bearer different-token"

though I'm not sure its necessary. My only concern is that with the current scalar nodeDistAuthHeader config is that if a malicious actor could inject a different nodeDistUrl, they could send the request to a malicious url and capture the token.

"npmMinimalAgeGate": {
"_package": "@yarnpkg/plugin-npm",
"title": "Minimum package version age required before installation.",
Expand Down