Skip to content

fix: Prevent Node subprocess from hanging during snyk test #6625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danskmt wants to merge 9 commits into
base: main
Choose a base branch
from
+113 −23
Open

fix: Prevent Node subprocess from hanging during snyk test #6625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d5c76ec
fix: Prevent Node subprocess from hanging during snyk test
danskmt Mar 6, 2026
5d70328
fix: Run prettier to fix lint issues
danskmt Mar 9, 2026
5e38091
Merge branch 'main' into fix/CLI-1166-node-subprocess-hangs-after-sny…
danskmt Mar 9, 2026
1e40976
Merge branch 'main' into fix/CLI-1166-node-sub...
danskmt Mar 9, 2026
38af1ce
fix: Update request tests to expect follow_max=0
danskmt Mar 9, 2026
2014c5c
fix: Preserve HTTP method and body on 307/308 redirects
danskmt Mar 9, 2026
aef5d2b
fix: Strip body headers on GET redirects to avoid server hangs
danskmt Mar 9, 2026
1d170b3
fix: Strip auth headers, guard malformed Location
danskmt Mar 9, 2026
39fc99a
fix: Deep copy headers, strip all sensitive headers
danskmt Mar 9, 2026
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
114 changes: 102 additions & 12 deletions src/lib/request/request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,25 +140,115 @@ function setupRequest(payload: Payload) {
return { method, url, data, options };
}

const REDIRECT_CODES = [301, 302, 303, 307, 308];
// 307/308 require preserving the original method and body per RFC 9110.
const METHOD_PRESERVING_REDIRECTS = [307, 308];
const MAX_REDIRECTS = 5;

export async function makeRequest(
payload: Payload,
): Promise<{ res: needle.NeedleResponse; body: any }> {
const { method, url, data, options } = setupRequest(payload);
// Disable needle's internal redirect following. When needle follows a
// redirect through a CONNECT proxy, the intermediate socket is never
// exposed to the callback and lingers, keeping the process alive.
// We follow redirects manually below so each hop gets its own agent
// and socket cleanup.
options.follow_max = 0;

return new Promise((resolve, reject) => {
needle.request(method, url, data, options, (err, res, respBody) => {
if (res?.headers?.[headerSnykAuthFailed] === 'true') {
return reject(new MissingApiTokenError());
}
// respBody potentially very large, do not output it in debug
debug('response (%s)', (res || {}).statusCode);
if (err) {
debug('response err: %s', err);
return reject(err);
}
let redirectsLeft = MAX_REDIRECTS;

resolve({ res, body: respBody });
});
const sendRequest = (
reqMethod: string,
reqUrl: string,
reqData: any,
reqOptions: needle.NeedleOptions,
) => {
needle.request(
reqMethod as needle.NeedleHttpVerbs,
reqUrl,
reqData,
reqOptions,
(err, res, respBody) => {
// Destroy the socket so the CONNECT tunnel doesn't keep the process alive.
res?.socket?.destroy();

if (res?.headers?.[headerSnykAuthFailed] === 'true') {
return reject(new MissingApiTokenError());
}
debug('response (%s)', (res || {}).statusCode);
if (err) {
debug('response err: %s', err);
return reject(err);
}

if (
res.statusCode &&
REDIRECT_CODES.includes(res.statusCode) &&
res.headers?.location &&
redirectsLeft > 0
) {
redirectsLeft--;

let redirectUrl: string;
try {
redirectUrl = new URL(
res.headers.location,
reqUrl,
).toString();
} catch (e) {
return reject(
new Error(`Invalid redirect Location: ${res.headers.location}`),
);
}

debug('following redirect to %s', redirectUrl);
const parsedRedirect = parse(redirectUrl);
const parsedOriginal = parse(reqUrl);
const newAgent =
parsedRedirect.protocol === 'http:'
? new http.Agent({ keepAlive: false })
: new https.Agent({ keepAlive: false });
const preserveMethod =
res.statusCode !== undefined &&
METHOD_PRESERVING_REDIRECTS.includes(res.statusCode);
const redirectHeaders = { ...reqOptions.headers };
if (!preserveMethod) {
delete redirectHeaders['content-length'];
delete redirectHeaders['content-encoding'];
}
if (parsedRedirect.host !== parsedOriginal.host) {
const sensitiveHeaders = [
'authorization',
'session-token',
'cookie',
'x-api-key',
'x-snyk-token',
];
for (const h of sensitiveHeaders) {
delete redirectHeaders[h];
}
}
const redirectOptions = {
...reqOptions,
agent: newAgent,
headers: redirectHeaders,
};
return sendRequest(
preserveMethod ? reqMethod : 'get',
redirectUrl,
preserveMethod ? reqData : null,
redirectOptions,
);
}

resolve({ res, body: respBody });
},
);
};

sendRequest(method, url, data, options);
});
}

Expand Down
22 changes: 11 additions & 11 deletions test/tap/request.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ test('request calls needle as expected and returns status code and body', (t) =>
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -85,7 +85,7 @@ test('request to localhost calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -124,7 +124,7 @@ test('request with timeout calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 100000, // provided
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -166,7 +166,7 @@ test('request with query string calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -205,7 +205,7 @@ test('request with json calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: false, // provided
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -247,7 +247,7 @@ test('request with custom header calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -286,7 +286,7 @@ test('request with https proxy calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.truthy,
Expand Down Expand Up @@ -335,7 +335,7 @@ test('request with http proxy calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.truthy,
Expand Down Expand Up @@ -375,7 +375,7 @@ test('request with no proxy calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -414,7 +414,7 @@ test('request with insecure calls needle as expected', (t) => {
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down Expand Up @@ -471,7 +471,7 @@ test('request calls needle as expected and will not update HTTP to HTTPS if envv
'content-encoding': undefined, // should not set when no data
'content-length': undefined, // should not be set when no data
}),
follow_max: 5, // default
follow_max: 0, // needle's redirect following is disabled; we handle redirects manually
timeout: 300000, // default
json: undefined, // default
agent: sinon.match.instanceOf(http.Agent),
Expand Down