feat(apps): introduce alternative node-runtime (PR 41019 replay) - #6
feat(apps): introduce alternative node-runtime (PR 41019 replay)#6tvnl-charan wants to merge 4 commits into
Conversation
refactor(apps): reduce node-runtime to a thin base adapter node-runtime now keeps only its platform seam: main.ts is a bootstrap that wires the stdout transport, the sandbox require (Node's global require) and empty sandbox globals, registers error listeners, then invokes the base message loop. error-handlers and stdoutTransport import the base messenger via the compiled specifier @rocket.chat/apps/base-runtime/dist/..., resolved by the existing loader-hook. Drop the stray debug console.error from loader-hook now that real runtime imports flow through it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> node-runtime main.ts comment chore(apps): include node-runtime in typecheck pipeline Standalone typecheck runs (yarn typecheck / CI) skipped node-runtime/src, so type errors there went uncaught. Add typecheck:node-runtime and wire it into the composite typecheck script. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LUT7EFw8g3GZjzD4huVpwD chore(apps): clean node-runtime/dist in build:clean The build:node-runtime step emits to node-runtime/dist, but build:clean did not remove it, leaving stale output across rebuilds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LUT7EFw8g3GZjzD4huVpwD fix(apps): print readable standalone-usage message in node-runtime console.error in Node formats a Uint8Array as a numeric byte dump rather than text, so the standalone-execution guard printed garbage. Pass the string directly, matching the deno-runtime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LUT7EFw8g3GZjzD4huVpwD
refactor(apps): make NodeSubprocessController use the BaseSubprocessController
docs(apps): fix typo in changeset (alernative -> alternative) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LUT7EFw8g3GZjzD4huVpwD
| export const sandboxRequire = (module: string) => { | ||
| // Normalize Node built-in specifiers: accept both 'crypto' and 'node:crypto' | ||
| const normalized = module.replace('node:', ''); | ||
|
|
||
| // We allow variants like 'fs', 'node:fs' or 'node:fs/promises', or even '@rocket.chat/apps-engine/**' | ||
| if (!ALLOWED_MODULES.some((mod) => normalized.startsWith(mod))) { | ||
| throw new Error(`Module ${module} is not allowed`); | ||
| } | ||
|
|
||
| // This is THE purpose of this function, we can't escape a dinamyc require call | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return, import/no-dynamic-require, @typescript-eslint/no-require-imports | ||
| return require(normalized); | ||
| }; |
There was a problem hiding this comment.
Sandbox Bypass and Remote Code Execution via Weak Module Validation in sandboxRequire
The sandboxRequire function uses an insecure startsWith check to validate allowed modules. This allows an attacker to bypass the sandbox by requiring arbitrary modules that share a prefix with an allowed module (e.g., fs-extra via fs) or by using path traversal to load arbitrary files from the filesystem when an allowed package prefix (e.g., @rocket.chat/apps-engine) is used. An attacker with the ability to deploy or run a custom application can leverage this to execute arbitrary code on the host system.
Steps to Reproduce
// Inside a sandboxed Rocket.Chat app:
const payload = require('@rocket.chat/apps-engine/../../../../../../tmp/malicious.js');
Fix with AI
A security vulnerability was found by Hacktron.
File: packages/apps/node-runtime/src/lib/require.ts
Lines: 25-37
Severity: high
Vulnerability: Sandbox Bypass and Remote Code Execution via Weak Module Validation in sandboxRequire
Description:
The `sandboxRequire` function uses an insecure `startsWith` check to validate allowed modules. This allows an attacker to bypass the sandbox by requiring arbitrary modules that share a prefix with an allowed module (e.g., `fs-extra` via `fs`) or by using path traversal to load arbitrary files from the filesystem when an allowed package prefix (e.g., `@rocket.chat/apps-engine`) is used. An attacker with the ability to deploy or run a custom application can leverage this to execute arbitrary code on the host system.
Proof of Concept:
// Inside a sandboxed Rocket.Chat app:
const payload = require('@rocket.chat/apps-engine/../../../../../../tmp/malicious.js');
Affected Code:
export const sandboxRequire = (module: string) => {
// Normalize Node built-in specifiers: accept both 'crypto' and 'node:crypto'
const normalized = module.replace('node:', '');
// We allow variants like 'fs', 'node:fs' or 'node:fs/promises', or even '@rocket.chat/apps-engine/**'
if (!ALLOWED_MODULES.some((mod) => normalized.startsWith(mod))) {
throw new Error(`Module ${module} is not allowed`);
}
// This is THE purpose of this function, we can't escape a dinamyc require call
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, import/no-dynamic-require, @typescript-eslint/no-require-imports
return require(normalized);
};
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), !accepted_risk <reason>, or !fixed (resolved). Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
| 'net', | ||
| 'http', | ||
| 'https', |
There was a problem hiding this comment.
Intentional Network Access in Apps-Engine Runtime
The node-runtime introduces a new sandbox implementation that explicitly allows apps to require sensitive Node.js modules, including net, http, and https. This allows any installed app to perform arbitrary network requests, leading to Server-Side Request Forgery (SSRF) and potential data exfiltration. The sandbox fails to restrict network access, which is a core requirement for a secure Apps-Engine environment.
Steps to Reproduce
An attacker can create a malicious Rocket.Chat app that uses `require('net')` or `require('http')` to make requests to internal services or external endpoints, bypassing any intended network restrictions. Example: `const http = require('http'); http.get('http://internal-service:8080/data', (res) => { ... });`
Fix with AI
A security vulnerability was found by Hacktron.
File: packages/apps/node-runtime/src/lib/require.ts
Lines: 7-9
Severity: info
Vulnerability: Intentional Network Access in Apps-Engine Runtime
Description:
The `node-runtime` introduces a new sandbox implementation that explicitly allows apps to `require` sensitive Node.js modules, including `net`, `http`, and `https`. This allows any installed app to perform arbitrary network requests, leading to Server-Side Request Forgery (SSRF) and potential data exfiltration. The sandbox fails to restrict network access, which is a core requirement for a secure Apps-Engine environment.
Proof of Concept:
An attacker can create a malicious Rocket.Chat app that uses `require('net')` or `require('http')` to make requests to internal services or external endpoints, bypassing any intended network restrictions. Example: `const http = require('http'); http.get('http://internal-service:8080/data', (res) => { ... });`
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), !accepted_risk <reason>, or !fixed (resolved). Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
| // The sandbox `require` handed to the app is Node's own global `require`; it | ||
| // needs no extra globals beyond the common ones the base eval shell binds. | ||
| setSandboxRequire(sandboxRequire); | ||
| setSandboxGlobals({}); |
There was a problem hiding this comment.
Sandbox Escape and Arbitrary Code Execution via Insecure Node.js Runtime Sandbox
Vulnerability Description
The newly introduced Node.js runtime (node-runtime) for Rocket.Chat Apps executes third-party app code using new Function in the main process context without any VM-level isolation or sandboxing.
While the Deno runtime can leverage Deno's native permission model (e.g., restricting file system and network access), Node.js has no such built-in restrictions. By using new Function to wrap and execute the app's code, the runtime exposes all Node.js global variables (such as process and global) to the executing app.
Exploitation Path
An attacker who is able to install or run a malicious Rocket.Chat App can easily escape the intended sandbox and execute arbitrary system commands on the host by accessing process.mainModule.require('child_process') or similar APIs.
Concrete Impact
This leads to complete compromise of the host system running the Rocket.Chat server (Remote Code Execution with the privileges of the Rocket.Chat process).
Steps to Reproduce
An attacker can install a malicious Rocket.Chat App containing the following code:
const payload = () => {
try {
const require = process.mainModule.require;
const execSync = require('child_process').execSync;
const output = execSync('id').toString();
console.error("EXPLOITED: " + output);
} catch (e) {
console.error("Failed to exploit: " + e.message);
}
};
payload();Fix with AI
A security vulnerability was found by Hacktron.
File: packages/apps/node-runtime/src/main.ts
Lines: 23-26
Severity: critical
Vulnerability: Sandbox Escape and Arbitrary Code Execution via Insecure Node.js Runtime Sandbox
Description:
### Vulnerability Description
The newly introduced Node.js runtime (`node-runtime`) for Rocket.Chat Apps executes third-party app code using `new Function` in the main process context without any VM-level isolation or sandboxing.
While the Deno runtime can leverage Deno's native permission model (e.g., restricting file system and network access), Node.js has no such built-in restrictions. By using `new Function` to wrap and execute the app's code, the runtime exposes all Node.js global variables (such as `process` and `global`) to the executing app.
### Exploitation Path
An attacker who is able to install or run a malicious Rocket.Chat App can easily escape the intended sandbox and execute arbitrary system commands on the host by accessing `process.mainModule.require('child_process')` or similar APIs.
### Concrete Impact
This leads to complete compromise of the host system running the Rocket.Chat server (Remote Code Execution with the privileges of the Rocket.Chat process).
Proof of Concept:
An attacker can install a malicious Rocket.Chat App containing the following code:
```javascript
const payload = () => {
try {
const require = process.mainModule.require;
const execSync = require('child_process').execSync;
const output = execSync('id').toString();
console.error("EXPLOITED: " + output);
} catch (e) {
console.error("Failed to exploit: " + e.message);
}
};
payload();
```
Affected Code:
// The sandbox `require` handed to the app is Node's own global `require`; it
// needs no extra globals beyond the common ones the base eval shell binds.
setSandboxRequire(sandboxRequire);
setSandboxGlobals({});
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), !accepted_risk <reason>, or !fixed (resolved). Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
| protected buildProcessConfiguration(): ProcessConfiguration { | ||
| const allowedDirs = [this.tempFilePath, path.resolve(path.dirname(this.scriptRuntimePath), '..', '..'), this.appsEnginePath]; | ||
|
|
||
| const args = [ | ||
| '--permission', | ||
| ...allowedDirs.map((dir) => `--allow-fs-read=${dir}`), | ||
| this.scriptRuntimePath, | ||
| '--subprocess', | ||
| this.appPackage.info.id, | ||
| '--spawnId', | ||
| String(this.spawnId++), | ||
| ]; | ||
|
|
||
| // SECURITY: We control the command, the arguments and the script that will be executed. | ||
| return { | ||
| command: this.nodeBin, | ||
| args, | ||
| options: { | ||
| env: { | ||
| PATH: process.env.PATH, | ||
| }, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Unrestricted Network Access in Node.js Apps-Engine Runtime Bypasses Permission Model
The Rocket.Chat Apps-Engine permission model is designed to restrict sandboxed applications from performing unauthorized actions, such as making network requests, unless they have explicitly requested and been granted the networking permission.
In the newly introduced Node.js runtime (AppsEngineNodeRuntime.ts), the subprocess is spawned using Node's experimental --permission flag. However, Node's permission model currently does not support restricting network access (there is no equivalent to Deno's --allow-net).
As a result, any application running under the Node.js runtime is granted full, unrestricted network access (outbound and inbound), completely ignoring whether the app requested the networking permission or not.
In contrast, the Deno runtime (AppsEngineDenoRuntime.ts) correctly checks the app's permissions and only appends the --allow-net flag if the networking permission is requested.
Steps to Reproduce
An app installed on a Rocket.Chat instance configured to use the Node.js runtime (`APPS_ENGINE_RUNTIME_BACKEND=node`) can perform outbound HTTP requests (e.g., using `http` or `https` modules) even if it has not requested the `networking` permission in its manifest.
Fix with AI
A security vulnerability was found by Hacktron.
File: packages/apps/src/server/runtime/node/AppsEngineNodeRuntime.ts
Lines: 19-42
Severity: high
Vulnerability: Unrestricted Network Access in Node.js Apps-Engine Runtime Bypasses Permission Model
Description:
The Rocket.Chat Apps-Engine permission model is designed to restrict sandboxed applications from performing unauthorized actions, such as making network requests, unless they have explicitly requested and been granted the `networking` permission.
In the newly introduced Node.js runtime (`AppsEngineNodeRuntime.ts`), the subprocess is spawned using Node's experimental `--permission` flag. However, Node's permission model currently does not support restricting network access (there is no equivalent to Deno's `--allow-net`).
As a result, any application running under the Node.js runtime is granted full, unrestricted network access (outbound and inbound), completely ignoring whether the app requested the `networking` permission or not.
In contrast, the Deno runtime (`AppsEngineDenoRuntime.ts`) correctly checks the app's permissions and only appends the `--allow-net` flag if the `networking` permission is requested.
Proof of Concept:
An app installed on a Rocket.Chat instance configured to use the Node.js runtime (`APPS_ENGINE_RUNTIME_BACKEND=node`) can perform outbound HTTP requests (e.g., using `http` or `https` modules) even if it has not requested the `networking` permission in its manifest.
Affected Code:
protected buildProcessConfiguration(): ProcessConfiguration {
const allowedDirs = [this.tempFilePath, path.resolve(path.dirname(this.scriptRuntimePath), '..', '..'), this.appsEnginePath];
const args = [
'--permission',
...allowedDirs.map((dir) => `--allow-fs-read=${dir}`),
this.scriptRuntimePath,
'--subprocess',
this.appPackage.info.id,
'--spawnId',
String(this.spawnId++),
];
// SECURITY: We control the command, the arguments and the script that will be executed.
return {
command: this.nodeBin,
args,
options: {
env: {
PATH: process.env.PATH,
},
},
};
}
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), !accepted_risk <reason>, or !fixed (resolved). Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
| if (!ALLOWED_MODULES.some((mod) => normalized.startsWith(mod))) { | ||
| throw new Error(`Module ${module} is not allowed`); | ||
| } |
There was a problem hiding this comment.
Weak Prefix Matching in sandboxRequire Allows Sandbox Escape and Path Traversal
The sandboxRequire function in packages/apps/node-runtime/src/lib/require.ts attempts to restrict the modules that a sandboxed Rocket.Chat app can import by checking if the requested module name starts with any of the allowed module names in ALLOWED_MODULES.
However, because it uses .startsWith(mod) without ensuring proper boundary separation (such as checking for equality, or verifying that the next character is / or :), it is vulnerable to prefix-matching bypasses and path traversal:
- Prefix-matching bypass: Any module name that begins with an allowed module name as a prefix will be permitted. For example, if
'fs'is allowed, then'fs-extra'(if installed) is also allowed. If'path'is allowed, then'path-to-regexp'is allowed. - Path traversal: An attacker can construct a path starting with an allowed module name followed by directory traversal sequences, such as
'@rocket.chat/apps-engine/../../package.json'. Since this starts with'@rocket.chat/apps-engine', it passes the validation check. Node'srequire()will then resolve the path relative to the@rocket.chat/apps-enginepackage, allowing the app to load and execute arbitrary JavaScript files within the allowed read directories (such as thenode-runtimepackage or other files in the monorepo).
Steps to Reproduce
An app running in the sandboxed Node.js runtime can execute:
const p = require('@rocket.chat/apps-engine/../../package.json');This will successfully bypass the allowed modules check and load the package.json file.
Fix with AI
A security vulnerability was found by Hacktron.
File: packages/apps/node-runtime/src/lib/require.ts
Lines: 30-32
Severity: high
Vulnerability: Weak Prefix Matching in sandboxRequire Allows Sandbox Escape and Path Traversal
Description:
The `sandboxRequire` function in `packages/apps/node-runtime/src/lib/require.ts` attempts to restrict the modules that a sandboxed Rocket.Chat app can import by checking if the requested module name starts with any of the allowed module names in `ALLOWED_MODULES`.
However, because it uses `.startsWith(mod)` without ensuring proper boundary separation (such as checking for equality, or verifying that the next character is `/` or `:`), it is vulnerable to prefix-matching bypasses and path traversal:
1. **Prefix-matching bypass**: Any module name that begins with an allowed module name as a prefix will be permitted. For example, if `'fs'` is allowed, then `'fs-extra'` (if installed) is also allowed. If `'path'` is allowed, then `'path-to-regexp'` is allowed.
2. **Path traversal**: An attacker can construct a path starting with an allowed module name followed by directory traversal sequences, such as `'@rocket.chat/apps-engine/../../package.json'`. Since this starts with `'@rocket.chat/apps-engine'`, it passes the validation check. Node's `require()` will then resolve the path relative to the `@rocket.chat/apps-engine` package, allowing the app to load and execute arbitrary JavaScript files within the allowed read directories (such as the `node-runtime` package or other files in the monorepo).
Proof of Concept:
An app running in the sandboxed Node.js runtime can execute:
```javascript
const p = require('@rocket.chat/apps-engine/../../package.json');
```
This will successfully bypass the allowed modules check and load the package.json file.
Affected Code:
if (!ALLOWED_MODULES.some((mod) => normalized.startsWith(mod))) {
throw new Error(`Module ${module} is not allowed`);
}
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), !accepted_risk <reason>, or !fixed (resolved). Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
Local replay of upstream RocketChat/Rocket.Chat PR RocketChat#41019 ("feat(apps): introduce alternative node-runtime"), the force-push scan (cfaa7f4 → 810ebbf).
hacktron-eval/apps-runtime-base@ 0759f83 (PR feat(apps): introduce alternative node-runtime RocketChat/Rocket.Chat#41019 base = develop)hacktron-eval/apps-runtime-head@ 810ebbf (force-pushed head)Raised to replay the "Missing Sandbox Isolation in Node Apps Runtime" finding (source main.ts, sink construct.ts) against the Hacktron engine.