Skip to content

Feature/upgrade runtime #3347

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
saecula wants to merge 6 commits into
base: develop
Choose a base branch
from
+417 −160
Open

Feature/upgrade runtime #3347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f048ad5
fix output-check tests
saecula Feb 5, 2026
1b1cfda
upgrade postman-runtime, re-add http/2 support + int tests
saecula Feb 5, 2026
5ed273b
fix security check, ignore claude dir
saecula Feb 6, 2026
e250918
rm overrides
saecula Feb 6, 2026
95be716
revert http/2 support
saecula Mar 17, 2026
bc01134
rm http/2 test
saecula Mar 17, 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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
strategy:
fail-fast: false
matrix:
node-version: [16, 18]
node-version: [18, 20, 22]
os: [ubuntu-latest, windows-latest]

steps:
Expand Down
6 changes: 5 additions & 1 deletion lib/run/secure-fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const fs = require('fs'),
FUNCTION = 'function',
DEPRECATED_SYNC_WRITE_STREAM = 'SyncWriteStream',
EXPERIMENTAL_PROMISE = 'promises',
DEPRECATED_FS_CONSTANTS = [
'F_OK', 'R_OK', 'W_OK', 'X_OK', 'COPYFILE_EXCL', 'COPYFILE_FICLONE', 'COPYFILE_FICLONE_FORCE'
],

// Use simple character check instead of regex to prevent regex attack
/*
Expand Down Expand Up @@ -143,7 +146,8 @@ SecureFS.prototype.resolvePathSync = function (relOrAbsPath, whiteList) {
// Attach all functions in fs to postman-fs
Object.getOwnPropertyNames(fs).map((prop) => {
// Bail-out early to prevent fs module from logging warning for deprecated and experimental methods
if (prop === DEPRECATED_SYNC_WRITE_STREAM || prop === EXPERIMENTAL_PROMISE || typeof fs[prop] !== FUNCTION) {
if (prop === DEPRECATED_SYNC_WRITE_STREAM || prop === EXPERIMENTAL_PROMISE ||
DEPRECATED_FS_CONSTANTS.includes(prop) || typeof fs[prop] !== FUNCTION) {
return;
}

Expand Down
29 changes: 27 additions & 2 deletions npm/test-cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,34 @@
return (file.substr(-8) === '.test.js');
}).forEach(mocha.addFile.bind(mocha));

// override exec for it to become silent by default
// override exec for it to become silent by default and filter deprecation warnings
global.exec = function (cmd, done) {
return exec(cmd, { silent: true }, done);
// Validate command starts with expected executables to prevent command injection
const allowedPrefixes = ['node ', 'newman ', './bin/newman'],
isAllowed = allowedPrefixes.some((prefix) => { return cmd.trim().startsWith(prefix); });

if (!isAllowed) {
const err = new Error(`Command not allowed: ${cmd}`);

return done ? done(1, '', err.message) : undefined;
}

return exec(cmd, { silent: true }, function (code, stdout, stderr) {

Check warning

Code scanning / CodeQL

Shell command built from environment values Medium test

This shell command depends on an uncontrolled
absolute path
Loading
.
This shell command depends on an uncontrolled
absolute path
Loading
.

Copilot Autofix

AI about 1 month ago

In general, the problem is that shelljs.exec is called with a single string cmd that was built by interpolating a path. The shell then parses cmd, so spaces or metacharacters in the path can alter the command. The standard fix is to avoid the shell for argument parsing: either (a) change callers to pass arguments separately and use an API that does not invoke a shell, or (b) at least escape or quote the interpolated values. Since we cannot touch the callers other than the shown test file, the best approach here is to (1) refactor the tests to use child_process.spawn/execFile directly with an argument array instead of global.exec, and (2) make global.exec in npm/test-cli.js reject multi-word shell commands and only allow a single binary path, effectively preventing unsafe templates from being used.

Concretely:

  • In test/cli/working-directory.test.js, stop calling global.exec with a template literal. Instead, require Node’s child_process and use spawn (or execFile) with:

    • command: 'node'
    • args: ['./bin/newman.js', 'run', 'test/fixtures/run/single-file-....json', '--working-dir', workingDir, ...]
      This passes workingDir as a separate argument and bypasses shell parsing entirely.

  • In npm/test-cli.js, harden the overridden global.exec so it only accepts a command that is a simple binary path with no spaces (or is already an array), and throws if the caller passes something that looks like a shell command line. This addresses the CodeQL sink at line 35 by ensuring we never pass an arbitrary space-containing string to shelljs.exec. For compatibility with existing tests that legitimately use a string binary path, we allow a single word (no whitespace) or, optionally, an object/array form; for now we’ll implement the strict single-token check. This keeps existing functionality for legitimate usages while blocking unsafe patterns like those in working-directory.test.js which we are refactoring away.

No new external packages are needed: child_process is built-in. The required changes are limited to the shown snippets in npm/test-cli.js and test/cli/working-directory.test.js.

Suggested changeset 2
npm/test-cli.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/npm/test-cli.js b/npm/test-cli.js
--- a/npm/test-cli.js
+++ b/npm/test-cli.js
@@ -32,6 +32,14 @@
 
         // override exec for it to become silent by default and filter deprecation warnings
         global.exec = function (cmd, done) {
+            // For safety, only allow simple commands without whitespace so that shell parsing
+            // cannot be influenced by untrusted or environment-derived values embedded in `cmd`.
+            // More complex invocations should use child_process.spawn/execFile with an args array.
+            if (typeof cmd === 'string' && /\s/.test(cmd)) {
+                throw new Error('Unsafe use of global.exec with a shell command string containing whitespace. ' +
+                    'Use child_process.spawn/execFile with argument arrays instead.');
+            }
+
             return exec(cmd, { silent: true }, function (code, stdout, stderr) {
                 // Filter out Node.js deprecation warnings from stderr (e.g., DEP0040 punycode warning in Node 22)
                 // This prevents test failures when Node emits deprecation warnings
EOF
@@ -32,6 +32,14 @@

// override exec for it to become silent by default and filter deprecation warnings
global.exec = function (cmd, done) {
// For safety, only allow simple commands without whitespace so that shell parsing
// cannot be influenced by untrusted or environment-derived values embedded in `cmd`.
// More complex invocations should use child_process.spawn/execFile with an args array.
if (typeof cmd === 'string' && /\s/.test(cmd)) {
throw new Error('Unsafe use of global.exec with a shell command string containing whitespace. ' +
'Use child_process.spawn/execFile with argument arrays instead.');
}

return exec(cmd, { silent: true }, function (code, stdout, stderr) {
// Filter out Node.js deprecation warnings from stderr (e.g., DEP0040 punycode warning in Node 22)
// This prevents test failures when Node emits deprecation warnings
test/cli/working-directory.test.js
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/cli/working-directory.test.js b/test/cli/working-directory.test.js
--- a/test/cli/working-directory.test.js
+++ b/test/cli/working-directory.test.js
@@ -1,20 +1,42 @@
 const path = require('path'),
     expect = require('chai').expect,
+    spawn = require('child_process').spawn,
 
     workingDir = path.resolve(__dirname, '../fixtures/files/work-dir');
 
 describe('newman run --working-dir --no-insecure-file-read', function () {
     it('should resolve file present inside working directory', function (done) {
-        // eslint-disable-next-line max-len
-        exec(`node ./bin/newman.js run test/fixtures/run/single-file-inside.json --working-dir ${workingDir}`, function (code) {
+        const child = spawn('node', [
+            './bin/newman.js',
+            'run',
+            'test/fixtures/run/single-file-inside.json',
+            '--working-dir',
+            workingDir
+        ], { shell: false });
+
+        child.on('close', function (code) {
             expect(code, 'should have exit code of 0').to.equal(0);
             done();
         });
     });
 
     it('should not resolve file present outside working directory with --no-insecure-file-read', function (done) {
-        // eslint-disable-next-line max-len
-        exec(`node ./bin/newman.js run test/fixtures/run/single-file-outside.json --working-dir ${workingDir} --no-insecure-file-read`, function (code, stdout) {
+        let stdout = '';
+
+        const child = spawn('node', [
+            './bin/newman.js',
+            'run',
+            'test/fixtures/run/single-file-outside.json',
+            '--working-dir',
+            workingDir,
+            '--no-insecure-file-read'
+        ], { shell: false });
+
+        child.stdout.on('data', function (data) {
+            stdout += data;
+        });
+
+        child.on('close', function (code) {
             expect(code, 'should have exit code of 1').to.equal(1);
             expect(stdout).to.have.string('AssertionError');
             done();
@@ -22,8 +33,21 @@
     });
 
     it('should resolve file present outside working directory by default', function (done) {
-        // eslint-disable-next-line max-len
-        exec(`node ./bin/newman.js run test/fixtures/run/single-file-outside.json --working-dir ${workingDir}`, function (code, stdout) {
+        let stdout = '';
+
+        const child = spawn('node', [
+            './bin/newman.js',
+            'run',
+            'test/fixtures/run/single-file-outside.json',
+            '--working-dir',
+            workingDir
+        ], { shell: false });
+
+        child.stdout.on('data', function (data) {
+            stdout += data;
+        });
+
+        child.on('close', function (code) {
             expect(code, 'should have exit code of 0').to.equal(0);
             expect(stdout).to.not.have.string('AssertionError');
             done();
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
@saecula saecula committed this autofix suggestion about 1 month ago.
Copy link
Collaborator Author

@saecula saecula Feb 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

this was already here, but added check for allowed prefixes to make it theoretically happier

// Filter out Node.js deprecation warnings from stderr (e.g., DEP0040 punycode warning in Node 22)
// This prevents test failures when Node emits deprecation warnings
const filteredStderr = stderr.split('\n')
.filter((line) => {
// Filter out deprecation warning lines and the follow-up trace hint
return !line.match(/^\(node:\d+\) \[DEP\d+\] DeprecationWarning:/) &&
!line.match(/^\(Use `node --trace-deprecation/);
})
.join('\n');

// Call the original callback with filtered stderr
if (done) {
done(code, stdout, filteredStderr);
}
});
};

// start the mocha run
Expand Down
Loading
Loading