diff --git a/README.md b/README.md index 88c98388f..9bde67e0a 100644 --- a/README.md +++ b/README.md @@ -280,6 +280,70 @@ const annotated = SandboxManager.annotateStderrWithSandboxFailures( ) ``` +**Asking about a host no rule decides (the ask callback).** The second argument to `SandboxManager.initialize` is an optional `SandboxAskCallback`. It is asked about a destination that neither `network.deniedDomains`, nor `network.allowedDomains`, nor a per-command allow list (next section) decided, and it is never asked under `network.strictAllowlist`. Without a callback such a destination is denied. The callback resolves to one of: + +- `true` - allow the connection. Only the value `true` allows. +- `false` - deny it. The violation line reports the generic reason `user denied`. +- `{ allow: false, reason: '...' }` - deny it, and report `reason` in the violation line in place of the generic text, so that whoever reads the `` block (a model included) learns why and what to do instead. + +Any other answer denies, whether it is truthy or not, and reports the generic reason. That includes an object that carries a `reason` without saying `allow: false`, such as `{ allow: true, reason: '...' }`: its reason is not reported. + +The reason is sanitized before it is stored, the way the rest of a violation line is. Each run of control characters (line breaks and tabs included) or of invisible ones (zero-width characters, the joiner among them, and bidi controls) becomes one space; `<` and `>` are removed, so `re-run with listed` is stored as `re-run with host listed`; the ends are trimmed. The result is cut to 500 characters, counted as UTF-16 code units the way `String.prototype.length` counts them. A reason with nothing left after that falls back to `user denied`. Write it as one line of plain text. + +**Behaviour change in the first release after 0.0.79:** the filter used to allow on any truthy answer, so a callback that resolved to a truthy value other than `true` (`1`, a string, any object) allowed the connection. It now denies. For the same reason, `{ allow: false, reason }` returned to an older release would be read there as an allow, because an object is truthy. `SandboxManager.askCallbackDenyReason` is `true` on a release that understands the object: check it before returning one, and deny with a plain `false` where it is absent. + +```typescript +const deny = (reason: string) => + SandboxManager.askCallbackDenyReason + ? { allow: false as const, reason } + : false + +await SandboxManager.initialize(config, async ({ host, port }) => { + if (await userApproves(host, port)) return true + return deny(`${host} was not approved for this session`) +}) +``` + +**Per-command network allow lists (`registerCommandNetworkLists`).** One invocation can carry an allow list of its own, in addition to the configured one, for as long as it runs: + +```typescript +import { randomBytes } from 'node:crypto' + +// 128 bits of randomness, 22 characters. See below for why nothing less will do. +const commandId = randomBytes(16).toString('base64url') +const wrapped = await SandboxManager.wrapWithSandbox( + command, + undefined, + undefined, + undefined, + { commandId }, +) + +SandboxManager.registerCommandNetworkLists(commandId, { + allowedDomains: ['registry.example.org', '*.cdn.example.org:443'], +}) +const child = spawn(wrapped, { shell: true }) +const unregister = () => SandboxManager.unregisterCommandNetworkLists(commandId) +child.once('exit', unregister) +child.once('error', unregister) // the child never started +``` + +Entries use the grammar and the matcher of `network.allowedDomains` and get the same validation. `registerCommandNetworkLists` throws on an invalid entry, and on a `commandId` shorter than 22 characters (an unpaired surrogate does not count). Registering an id again replaces its list; `unregisterCommandNetworkLists` is a no-op for an id that has none; `reset()` removes every registration. + +The order of evaluation for a connection is: + +1. a `network.deniedDomains` match denies; +2. a `network.allowedDomains` match allows; +3. `network.strictAllowlist` denies; +4. a match in the list registered for the id the connection presents allows; +5. the ask callback decides if there is one, and otherwise the connection is denied. + +So a per-command entry never overrides a configured `deniedDomains` entry, and is ignored entirely under `strictAllowlist`. It never enters `network.*` either: `getConfig()` and `getNetworkRestrictionConfig()` do not show it, and the default `injectHosts` scope of a masked credential, which is `network.allowedDomains`, does not grow. A host allowed this way still goes through the resolved-address check when it is dialed, and an IP literal in a per-command list adds no exemption there. + +What the id has to be, and what this feature is not: the proxy learns which invocation a connection belongs to from the proxy username, and the username is presented by the client inside the sandbox. The id is therefore the **only** thing binding a connection to an allow list. It **must** be unguessable: at least 128 bits of randomness, never a counter, a timestamp or anything derived from the command. A sandboxed process that presents another live invocation's id gets that invocation's allows, so this is attribution, not a boundary between concurrent commands of one session. Register just before spawning the wrapped command, and unregister when the child exits or never started: a registration that outlives its command widens the window in which its id is worth presenting. As with every attribution key, only the first 100 characters of an id take part. Keep ids ASCII: a list under a non-ASCII id whose encoded form does not fit in the proxy username (255 bytes) registers and never applies. + +No list applies while `network.httpProxyPort` names an external proxy: the wrap gives that proxy no username, so no connection carries an id. `registerCommandNetworkLists` still succeeds (and says so in one line under `SRT_DEBUG`), and every connection is decided as if no list existed. + #### Available exports ```typescript diff --git a/src/sandbox/domain-pattern.ts b/src/sandbox/domain-pattern.ts index 8019d6c79..502c5f5ea 100644 --- a/src/sandbox/domain-pattern.ts +++ b/src/sandbox/domain-pattern.ts @@ -3,6 +3,9 @@ * (sandbox-manager) and config-time validation (sandbox-config). * Lives in its own module so the schema can import it without pulling * in sandbox-manager (which imports the schema — circular). + * + * The validity check for a list entry lives here too, so the manager can + * apply it to lists registered at run time without loading the schema module. */ import { isIP } from 'node:net' @@ -73,6 +76,83 @@ function parsePortSuffix(suffix: string): number | undefined { return port > 65535 ? undefined : port } +/** + * Host-only pattern check (e.g., "example.com", "*.npmjs.org"). Rejects + * protocols, paths, ports, and overly broad wildcards. + */ +export function isValidDomainPattern(val: string): boolean { + // A bare IPv6 literal as produced by splitDomainPatternPort for a + // bracketed entry (`[::1]`, `[2001:db8::1]:443` -> `::1`, `2001:db8::1`). + // Whether the *raw* entry was bracketed is enforced separately + // (hasValidIpv6Bracketing) before the split. + if (isIP(val) === 6) return true + + // Reject protocols, paths, ports, etc. + if (val.includes('://') || val.includes('/') || val.includes(':')) { + return false + } + + // Allow localhost + if (val === 'localhost') return true + + // Allow wildcard domains like *.example.com + if (val.startsWith('*.')) { + const domain = val.slice(2) + // After the *. there must be a valid domain with at least one more dot + // e.g., *.example.com is valid, *.com is not (too broad) + if ( + !domain.includes('.') || + domain.startsWith('.') || + domain.endsWith('.') + ) { + return false + } + // Count dots - must have at least 2 parts after the wildcard (e.g., example.com) + const parts = domain.split('.') + return parts.length >= 2 && parts.every(p => p.length > 0) + } + + // Reject any other use of wildcards (e.g., *, *., etc.) + if (val.includes('*')) { + return false + } + + // Regular domains must have at least one dot and only valid characters + return val.includes('.') && !val.startsWith('.') && !val.endsWith('.') +} + +export const DOMAIN_PATTERN_MESSAGE = + 'Invalid domain pattern. Must be a valid domain (e.g., "example.com"), a wildcard (e.g., "*.example.com"), or a bracketed IPv6 literal (e.g., "[::1]", "[2001:db8::1]:443"). Overly broad patterns like "*.com" or "*" are not allowed for security reasons.' + +/** + * Raw-entry rule applied before the port split: an entry with two or more + * colons is an IPv6 literal and must use RFC 3986 brackets (`[::1]`, + * `[::1]:443`). Unbracketed it is ambiguous (`2001:db8::1:443` is itself a + * valid 8-hextet address), so reject it and make the user say which they + * mean, rather than accept an entry that can silently match the wrong thing. + */ +export function hasValidIpv6Bracketing(val: string): boolean { + const first = val.indexOf(':') + const multiColon = first !== -1 && val.indexOf(':', first + 1) !== -1 + return !multiColon || val.startsWith('[') +} + +/** + * The check an allow-list entry gets: a domain pattern with an optional + * `:port` suffix. Shared by the `network.allowedDomains` schema and the lists + * registered at run time, so neither accepts what the other refuses. + */ +export function isValidAllowedDomainEntry(val: string): boolean { + return ( + hasValidIpv6Bracketing(val) && + isValidDomainPattern(splitDomainPatternPort(val).hostPattern) + ) +} + +export const ALLOWED_DOMAIN_ENTRY_MESSAGE = + DOMAIN_PATTERN_MESSAGE + + ' An optional ":port" suffix (1-65535) restricts the entry to that port.' + /** * The IP-literal entries of an allow/deny list as `{ range, port? }` rules * for the resolved-address check: an address the list names explicitly is diff --git a/src/sandbox/sandbox-config.ts b/src/sandbox/sandbox-config.ts index a26d69924..74d1672f6 100644 --- a/src/sandbox/sandbox-config.ts +++ b/src/sandbox/sandbox-config.ts @@ -3,7 +3,6 @@ * This is the main configuration interface that consumers pass to SandboxManager.initialize() */ -import { isIP } from 'node:net' import type { FilterRequestCallback } from './request-filter.js' import { isAbsolute, posix as posixPath, win32 as win32Path } from 'node:path' @@ -13,7 +12,12 @@ import { isAbsolute, posix as posixPath, win32 as win32Path } from 'node:path' // schemas the v4 API they are not written against. import { z } from 'zod/v3' import { + ALLOWED_DOMAIN_ENTRY_MESSAGE, + DOMAIN_PATTERN_MESSAGE, + hasValidIpv6Bracketing, isInjectHostCoveredByAllowedDomains, + isValidAllowedDomainEntry, + isValidDomainPattern, splitDomainPatternPort, stripDomainPatternPort, } from './domain-pattern.js' @@ -21,54 +25,6 @@ import { parseAddressRange } from './address.js' import { containsGlobCharsForPlatform } from './sandbox-utils.js' import { getPlatform } from '../utils/platform.js' -/** - * Host-only pattern check (e.g., "example.com", "*.npmjs.org"). Rejects - * protocols, paths, ports, and overly broad wildcards. - */ -function isValidDomainPattern(val: string): boolean { - // A bare IPv6 literal as produced by splitDomainPatternPort for a - // bracketed entry (`[::1]`, `[2001:db8::1]:443` → `::1`, `2001:db8::1`). - // Whether the *raw* entry was bracketed is enforced separately - // (hasValidIpv6Bracketing) before the split. - if (isIP(val) === 6) return true - - // Reject protocols, paths, ports, etc. - if (val.includes('://') || val.includes('/') || val.includes(':')) { - return false - } - - // Allow localhost - if (val === 'localhost') return true - - // Allow wildcard domains like *.example.com - if (val.startsWith('*.')) { - const domain = val.slice(2) - // After the *. there must be a valid domain with at least one more dot - // e.g., *.example.com is valid, *.com is not (too broad) - if ( - !domain.includes('.') || - domain.startsWith('.') || - domain.endsWith('.') - ) { - return false - } - // Count dots - must have at least 2 parts after the wildcard (e.g., example.com) - const parts = domain.split('.') - return parts.length >= 2 && parts.every(p => p.length > 0) - } - - // Reject any other use of wildcards (e.g., *, *., etc.) - if (val.includes('*')) { - return false - } - - // Regular domains must have at least one dot and only valid characters - return val.includes('.') && !val.startsWith('.') && !val.endsWith('.') -} - -const DOMAIN_PATTERN_MESSAGE = - 'Invalid domain pattern. Must be a valid domain (e.g., "example.com"), a wildcard (e.g., "*.example.com"), or a bracketed IPv6 literal (e.g., "[::1]", "[2001:db8::1]:443"). Overly broad patterns like "*.com" or "*" are not allowed for security reasons.' - /** * Schema for domain patterns (e.g., "example.com", "*.npmjs.org") * Validates that domain patterns are safe and don't include overly broad wildcards @@ -81,32 +37,11 @@ const domainPatternSchema = z * Domain pattern with an optional `:port` suffix (e.g., "example.com:443", * "*.npmjs.org:8443"). Used for allowedDomains / deniedDomains, where the * proxy knows the destination port; an entry without a port matches any port. + * The check and its message live in domain-pattern.ts, which says why. */ -/** - * Raw-entry rule applied before the port split: an entry with two or more - * colons is an IPv6 literal and must use RFC 3986 brackets (`[::1]`, - * `[::1]:443`). Unbracketed it is ambiguous — `2001:db8::1:443` is itself a - * valid 8-hextet address — so reject it and make the user say which they - * mean, rather than accept an entry that can silently match the wrong thing. - */ -function hasValidIpv6Bracketing(val: string): boolean { - const first = val.indexOf(':') - const multiColon = first !== -1 && val.indexOf(':', first + 1) !== -1 - return !multiColon || val.startsWith('[') -} - const domainPortPatternSchema = z .string() - .refine( - val => - hasValidIpv6Bracketing(val) && - isValidDomainPattern(splitDomainPatternPort(val).hostPattern), - { - message: - DOMAIN_PATTERN_MESSAGE + - ' An optional ":port" suffix (1-65535) restricts the entry to that port.', - }, - ) + .refine(isValidAllowedDomainEntry, { message: ALLOWED_DOMAIN_ENTRY_MESSAGE }) /** * deniedDomains entry: a domainPortPattern, or a bare "*" / "*:port" @@ -764,7 +699,7 @@ export const NetworkConfigSchema = z.object({ .boolean() .optional() .describe( - 'If true, hosts not in allowedDomains are denied without consulting the ask callback. Set this when allowedDomains is policy enforcement, not a prompt-suppression hint.', + 'If true, hosts not in allowedDomains are denied without consulting the ask callback, and a per-command allow list (registerCommandNetworkLists) is ignored. Set this when allowedDomains is policy enforcement, not a prompt-suppression hint.', ), deniedResolvedAddresses: z .array(addressRangeSchema) diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index a113df317..93b029138 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -110,6 +110,7 @@ import { } from './sandbox-utils.js' import { SandboxViolationStore, + sanitizeDenialReason, sanitizeUnregisteredCommandKey, shouldIgnoreViolation, } from './sandbox-violation-store.js' @@ -122,6 +123,8 @@ import { resolveParentProxy, } from './parent-proxy.js' import { + ALLOWED_DOMAIN_ENTRY_MESSAGE, + isValidAllowedDomainEntry, matchesDomainPattern, matchesDomainPatternWithPort, stripDomainPatternPort, @@ -274,6 +277,82 @@ export function resolveCommandText(decodedKey: string): string { ) } +/** + * The shortest commandId a per-command allow list may be registered under: + * 16 random bytes (128 bits) as unpadded base64url. The floor only turns away + * what is too short to hold 128 bits: it cannot tell a random id from a + * predictable one of the same length. + */ +const MIN_NETWORK_LISTS_COMMAND_ID_LENGTH = 22 + +/** + * Attribution key to the network allow list registered for one invocation + * (registerCommandNetworkLists), keyed the way commandTextsByKey is because + * the proxy username delivers both. The embedder removes an entry when its + * command is gone. Not bounded the way commandTextsByKey is: dropping the + * oldest entry would silently take a list away from a running command. + */ +const commandNetworkListsByKey = new Map() + +function registerCommandNetworkLists( + commandId: string, + lists: { allowedDomains?: string[] }, +): void { + // The floor is measured on the key the map holds, not on the raw id, and + // less U+FFFD: every unpaired surrogate becomes one, so it carries nothing. + const key = + typeof commandId === 'string' + ? decodeSandboxedCommand(encodeSandboxedCommand(commandId)) + : '' + // The id is not echoed in either message: it is what stands between a + // sandboxed process and this list, and error text ends up in logs. + if (key.replace(/\ufffd/g, '').length < MIN_NETWORK_LISTS_COMMAND_ID_LENGTH) { + throw new Error( + `registerCommandNetworkLists: commandId must be a string of at least ` + + `${MIN_NETWORK_LISTS_COMMAND_ID_LENGTH} characters, unpaired ` + + `surrogates not counted. The id is the ` + + `only thing that binds a proxy connection to this allow list, and ` + + `the sandboxed process chooses which id it presents, so it must be ` + + `random (at least 128 bits, e.g. 16 random bytes as base64url) and ` + + `never derived from the command text, a counter or the time.`, + ) + } + const allowedDomains = lists?.allowedDomains ?? [] + if (!Array.isArray(allowedDomains)) { + throw new Error( + 'registerCommandNetworkLists: allowedDomains must be an array of strings.', + ) + } + for (const entry of allowedDomains) { + if (typeof entry !== 'string' || !isValidAllowedDomainEntry(entry)) { + throw new Error( + `registerCommandNetworkLists: invalid allowedDomains entry ` + + `${JSON.stringify(entry)}. ${ALLOWED_DOMAIN_ENTRY_MESSAGE}`, + ) + } + } + commandNetworkListsByKey.set( + key, + // Copied, so the caller cannot change what was validated. + { allowedDomains: [...allowedDomains] }, + ) + // An external HTTP proxy is handed no username, so no connection names an id. + if (config?.network.httpProxyPort !== undefined) { + logForDebugging( + 'registerCommandNetworkLists: network.httpProxyPort is set (an ' + + 'external proxy), so this list will never apply', + { level: 'warn' }, + ) + } +} + +function unregisterCommandNetworkLists(commandId: string): void { + if (typeof commandId !== 'string') return + commandNetworkListsByKey.delete( + decodeSandboxedCommand(encodeSandboxedCommand(commandId)), + ) +} + /** * Record a proxy-side denial in the violation store so the model sees a * structured block alongside the raw 403 / SOCKS @@ -352,6 +431,29 @@ function redactUrlForViolation(url: string): string { } } +/** Whether an ask-callback answer is one of the two documented denials. */ +function isDeclaredDenial(answer: unknown): boolean { + return ( + answer === false || + (typeof answer === 'object' && + answer !== null && + (answer as { allow?: unknown }).allow === false) + ) +} + +/** + * The reason an ask-callback answer gives for denying, sanitized, or + * undefined when it gives none that can be shown. Only + * `{ allow: false, reason }` supplies one: a reason on any other object was + * not written as the explanation of a denial. + */ +function denialReasonOf(answer: unknown): string | undefined { + if (typeof answer !== 'object' || answer === null) return undefined + const { allow, reason } = answer as { allow?: unknown; reason?: unknown } + if (allow !== false || typeof reason !== 'string') return undefined + return sanitizeDenialReason(reason) || undefined +} + async function filterNetworkRequest( port: number, host: string, @@ -408,22 +510,51 @@ async function filterNetworkRequest( } } - // No matching rules - ask user or deny. strictAllowlist makes the - // allowlist deterministic enforcement: never fall through to the callback. - if (!sandboxAskCallback || config.network.strictAllowlist) { + // strictAllowlist makes the configured allowlist deterministic enforcement: + // neither a per-command allow list nor the callback below can widen it. + if (config.network.strictAllowlist) { logForDebugging(`No matching config rule, denying: ${host}:${port}`) return denied('host is not on the allow list') } - logForDebugging(`No matching config rule, asking user: ${host}:${port}`) + // The allow list registered for the invocation the proxy username names. + // Coming after both configured lists, it lifts the default deny for that + // one command and never a configured deniedDomains entry. The username is + // presented by the sandboxed client: see registerCommandNetworkLists. + const perCommand = encodedCommand + ? commandNetworkListsByKey.get(decodeSandboxedCommand(encodedCommand)) + : undefined + for (const allowedDomain of perCommand?.allowedDomains ?? []) { + if (matchesDomainPatternWithPort(canonicalHost, port, allowedDomain)) { + logForDebugging(`Allowed by per-command rule: ${host}:${port}`) + return true + } + } + + if (!sandboxAskCallback) { + logForDebugging(`No matching rule, denying: ${host}:${port}`) + return denied('host is not on the allow list') + } + + logForDebugging(`No matching rule, asking user: ${host}:${port}`) try { - const userAllowed = await sandboxAskCallback({ host, port }) - if (userAllowed) { + // Held as unknown: a JavaScript caller is not bound by the declared type. + const answer: unknown = await sandboxAskCallback({ host, port }) + // Only `true` allows: an object is how a callback denies with a reason, + // and a truthiness test here would read every such denial as an allow. + if (answer === true) { logForDebugging(`User allowed: ${host}:${port}`) return true } + if (!isDeclaredDenial(answer)) { + logForDebugging( + `Ask callback answered neither a boolean nor { allow: false, reason }; treating it as a denial: ${host}:${port}`, + { level: 'warn' }, + ) + } logForDebugging(`User denied: ${host}:${port}`) - return denied('user denied') + // The callback's own reason, when it gave one, says what to do instead. + return denied(denialReasonOf(answer) ?? 'user denied') } catch (error) { logForDebugging(`Error in permission callback: ${error}`, { level: 'error', @@ -1704,6 +1835,9 @@ export type WrapWithSandboxOptions = { * command text: keys are compared on their first 100 characters, so two * long commands sharing a prefix would otherwise cross-attribute, and a * rerun of the same text would inherit the earlier run's events. + * + * An id that a network allow list is registered under must also be + * unguessable: see `registerCommandNetworkLists`. */ commandId?: string /** @@ -2533,6 +2667,7 @@ async function reset(): Promise { sentinelRegistry.clear() awsPairRegistry.clear() commandTextsByKey.clear() + commandNetworkListsByKey.clear() maskedFileStore.dispose() } @@ -2673,6 +2808,60 @@ export interface ISandboxManager { cwd?: string, options?: WrapWithSandboxOptions, ): Promise<{ argv: string[]; env: NodeJS.ProcessEnv }> + /** + * Register the network allow list of one invocation, under the `commandId` + * its wrap was given ({@link WrapWithSandboxOptions}). Entries are matched + * and validated as `network.allowedDomains` entries are; an invalid one + * throws. Registering an id again replaces its list. + * + * Order of evaluation for a connection: a configured `deniedDomains` match + * denies; a configured `allowedDomains` match allows; + * `network.strictAllowlist` denies; a match in the list registered for the + * id the connection presents allows; then the ask callback decides if + * there is one, and otherwise the connection is denied. So a per-command + * entry never overrides a configured `deniedDomains` entry and is ignored + * entirely under `strictAllowlist`. It also never enters `network.*`, so + * the default `injectHosts` scope of a masked credential does not grow. + * + * The proxy reads the id from the proxy username, which the sandboxed + * client presents, so the id is the ONLY thing binding a connection to an + * allow list. It MUST be unguessable: at least 128 bits of randomness (16 + * random bytes as base64url, 22 characters; a shorter id throws), never a + * counter, a timestamp or anything derived from the command. Presenting + * another live invocation's id gets that invocation's allows: this is + * attribution, not a boundary between concurrent commands of one session. + * + * Register just before spawning the wrapped command, and unregister when + * the child exits or never started: a registration that outlives its + * command widens the window in which its id is worth presenting. + * + * As with every attribution key, only the first 100 characters of an id + * take part, and a list under an id whose encoded form does not fit in the + * proxy username (possible only with non-ASCII ids) never applies. An + * unpaired surrogate does not count towards the 22 characters. + * + * No list applies while `network.httpProxyPort` names an external proxy: + * the wrap gives that proxy no username, so no connection carries an id. + * Registering still succeeds, and every connection is decided as if no list + * existed. + */ + registerCommandNetworkLists( + commandId: string, + lists: { allowedDomains?: string[] }, + ): void + /** + * Remove the list registered for `commandId`. A no-op for an id that has + * none. `reset()` removes them all. + */ + unregisterCommandNetworkLists(commandId: string): void + /** + * True when the network filter understands an ask callback's + * `{ allow: false, reason }` answer (see {@link SandboxAskCallback}). Read + * it before returning that object: releases up to v0.0.77 allow on any + * truthy answer, so there the object would be read as an allow. Where this + * is absent, deny with a plain `false`. + */ + readonly askCallbackDenyReason: true getSandboxViolationStore(): SandboxViolationStore annotateStderrWithSandboxFailures(command: string, stderr: string): string getLinuxGlobPatternWarnings(): string[] @@ -2716,6 +2905,9 @@ export const SandboxManager: ISandboxManager = { waitForNetworkInitialization, wrapWithSandbox, wrapWithSandboxArgv, + registerCommandNetworkLists, + unregisterCommandNetworkLists, + askCallbackDenyReason: true, cleanupAfterCommand, reset, getMitmCA: () => mitmCA, diff --git a/src/sandbox/sandbox-schemas.ts b/src/sandbox/sandbox-schemas.ts index 09ac5a30b..491cc16c6 100644 --- a/src/sandbox/sandbox-schemas.ts +++ b/src/sandbox/sandbox-schemas.ts @@ -104,9 +104,11 @@ export interface CredentialRestrictionConfig { * * Note: Empty `allowedHosts` means no host matches an allow rule (unlike * read's empty denyOnly). Whether an unmatched host is denied outright - * depends on the ask callback: deniedHosts are checked first and deny - * unconditionally; a host matching neither list falls through to the - * registered SandboxAskCallback when one exists, and is denied only when + * depends on what else may decide it: deniedHosts are checked first and deny + * unconditionally; a host matching neither list is denied under + * `network.strictAllowlist`, else allowed by a matching per-command list + * (`SandboxManager.registerCommandNetworkLists`), else falls through to the + * registered SandboxAskCallback when one exists, and is denied when * no callback is registered. Hosts needing a hard block-all regardless of * callback behavior should use a `deniedHosts` wildcard. * @@ -126,6 +128,22 @@ export type NetworkHostPattern = { port: number | undefined } +/** + * Asked about a host that no configured rule and no per-command allow list + * decided (never asked under `network.strictAllowlist`). Only `true` allows. + * `{ allow: false, reason }` denies with that reason, which the violation + * line reports. `false` and every other answer, truthy or not, deny with the + * generic reason "user denied": a `reason` on an object that does not say + * `allow: false` is not reported. + * + * The reason is sanitized like the rest of a violation line (runs of control + * or invisible characters become one space, `<` and `>` are removed, the + * ends are trimmed) and cut to 500 UTF-16 code units, so write it as one + * line of plain text. With nothing left, "user denied" is reported. + * + * Check `SandboxManager.askCallbackDenyReason` before returning an object: + * a release without it reads any truthy answer as an allow. + */ export type SandboxAskCallback = ( params: NetworkHostPattern, -) => Promise +) => Promise diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index e685f2dc6..a7dde8753 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -1103,12 +1103,14 @@ export function proxyUsernameFor(encodedCommand: string | undefined): string { /** * Inverse of {@link proxyUsernameFor}: extract the encodedCommand suffix * from `srt.`, or undefined for bare `srt` / anything else. - * The username is client-controlled inside the sandbox, so a forged suffix - * can only misattribute a denial in the violation report — it cannot - * authenticate (the token does that) or reach another command's data. A + * The username is presented by the client inside the sandbox. A forged + * suffix cannot authenticate (the token does that), but it decides which + * invocation a denial is attributed to in the violation report and which + * registered network allow list applies to the connection: see + * `SandboxManager.registerCommandNetworkLists` for what that asks of an id. A * suffix past {@link MAX_ENCODED_COMMAND_BYTES} is longer than this process * can mint, so it is dropped rather than stored: the denial is still - * recorded, unattributed. + * recorded, unattributed, and no registered list applies. */ export function encodedCommandFromProxyUser( username: string | undefined, diff --git a/src/sandbox/sandbox-violation-store.ts b/src/sandbox/sandbox-violation-store.ts index afc4ac297..eccda8e8f 100644 --- a/src/sandbox/sandbox-violation-store.ts +++ b/src/sandbox/sandbox-violation-store.ts @@ -143,10 +143,34 @@ function sanitizeViolationText(text: string): string { * that (an HTTP proxy header budget is kilobytes). */ export function sanitizeUnregisteredCommandKey(decodedKey: string): string { - const cut = sanitizeViolationText(decodedKey).slice( - 0, + return cutBetweenCharacters( + sanitizeViolationText(decodedKey), SANDBOXED_COMMAND_KEY_LENGTH, ) +} + +/** + * How much of a denial reason supplied by the ask callback a violation line + * carries, in UTF-16 code units: a few sentences fit, a document does not. + */ +export const MAX_DENIAL_REASON_LENGTH = 500 + +/** + * A denial reason the ask callback supplied, as a violation line may carry + * it: sanitized first, so the cut counts what is displayed, then cut so that + * one answer cannot flood the text a model reads. The end is trimmed after + * the cut: the reason sits inside parentheses, where the store's own trim of + * the line's ends does not reach. + */ +export function sanitizeDenialReason(reason: string): string { + return cutBetweenCharacters( + sanitizeViolationText(reason), + MAX_DENIAL_REASON_LENGTH, + ).trimEnd() +} + +function cutBetweenCharacters(text: string, length: number): string { + const cut = text.slice(0, length) // A cut by UTF-16 code unit can land between a surrogate pair; the lone // high surrogate left behind renders as a replacement character. return /[\uD800-\uDBFF]$/.test(cut) ? cut.slice(0, -1) : cut diff --git a/test/sandbox/update-config.test.ts b/test/sandbox/update-config.test.ts index f10dfa1fc..fc3ef856f 100644 --- a/test/sandbox/update-config.test.ts +++ b/test/sandbox/update-config.test.ts @@ -1,7 +1,17 @@ -import { describe, it, expect, afterEach, beforeEach } from 'bun:test' -import { SandboxManager } from '../../src/index.js' -import { connect } from 'net' +import { describe, it, expect, afterEach, beforeEach, spyOn } from 'bun:test' +import { + SandboxManager, + SandboxRuntimeConfigSchema, + type SandboxAskCallback, + type SandboxRuntimeConfig, +} from '../../src/index.js' +import { connect, createServer, type AddressInfo, type Server } from 'net' +import { randomBytes } from 'node:crypto' import { getPlatform } from '../../src/utils/platform.js' +import { + encodeSandboxedCommand, + proxyUsernameFor, +} from '../../src/sandbox/sandbox-utils.js' import { spawnAsync } from '../helpers/spawn.js' import { isLinux, isMacOS } from '../helpers/platform.js' @@ -387,6 +397,896 @@ describe('proxy auth + network deny semantics', () => { }) }) +/** + * CONNECT `target` ("host:port") through the proxy under `username`, with the + * session token, and report the status code of the reply. + */ +function connectAs( + proxyPort: number, + target: string, + username: string, +): Promise { + return new Promise(resolve => { + const token = SandboxManager.getProxyAuthToken() + const auth = Buffer.from(`${username}:${token}`).toString('base64') + const socket = connect(proxyPort, '127.0.0.1', () => { + socket.write( + `CONNECT ${target} HTTP/1.1\r\nHost: ${target}\r\n` + + `Proxy-Authorization: Basic ${auth}\r\n\r\n`, + ) + }) + let data = '' + socket.on('data', chunk => { + data += chunk.toString() + if (data.includes('\r\n')) { + socket.destroy() + const status = data.match(/HTTP\/1\.\d (\d+)/) + resolve(status ? parseInt(status[1]) : undefined) + } + }) + socket.on('error', () => resolve(undefined)) + socket.setTimeout(2000, () => { + socket.destroy() + resolve(undefined) + }) + }) +} + +/** + * The same over SOCKS5: the reply code to a CONNECT for `target` under + * `username` (0 is granted, 2 is refused by the rules). + */ +function socksConnectAs( + proxyPort: number, + target: string, + username: string, +): Promise { + return new Promise(resolve => { + const [host, port] = target.split(':') as [string, string] + const field = (text: string): Buffer => + Buffer.concat([Buffer.from([Buffer.byteLength(text)]), Buffer.from(text)]) + const socket = connect(proxyPort, '127.0.0.1', () => { + socket.write( + Buffer.concat([ + // Offer username/password only, authenticate, then CONNECT by name. + Buffer.from([0x05, 0x01, 0x02, 0x01]), + field(username), + field(SandboxManager.getProxyAuthToken() ?? ''), + Buffer.from([0x05, 0x01, 0x00, 0x03]), + field(host), + Buffer.from([Number(port) >> 8, Number(port) & 0xff]), + ]), + ) + }) + let reply = Buffer.alloc(0) + socket.on('data', chunk => { + reply = Buffer.concat([reply, chunk]) + // Method selection (2 bytes), auth status (2), then VER and REP. + if (reply.length >= 6) socket.destroy() + }) + socket.on('close', () => resolve(reply[5])) + socket.on('error', () => {}) + socket.setTimeout(2000, () => socket.destroy()) + }) +} + +/** + * A listener on loopback for the proxy to dial, so an allow is observed as a + * 200 without leaving the machine: an IP literal has no name to resolve and + * nothing for the resolved-address check to refuse. It answers a request with + * the body `ok`. + */ +function listenOnLoopback(): Promise<{ server: Server; target: string }> { + return new Promise(resolve => { + const server = createServer(socket => { + socket.on('error', () => {}) + socket.once('data', () => + socket.end('HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok'), + ) + }) + server.listen(0, '127.0.0.1', () => { + const { port } = server.address() as AddressInfo + resolve({ server, target: `127.0.0.1:${port}` }) + }) + }) +} + +/** An id of the kind the API asks for: 16 random bytes, 22 characters. */ +const randomCommandId = (): string => randomBytes(16).toString('base64url') + +/** The proxy username a wrap given this commandId puts in the proxy URLs. */ +const usernameFor = (commandId: string): string => + proxyUsernameFor(encodeSandboxedCommand(commandId)) + +const NO_FILESYSTEM_RULES = { denyRead: [], allowWrite: [], denyWrite: [] } + +const violationLines = (): string[] => + SandboxManager.getSandboxViolationStore() + .getViolations() + .map(v => v.line) + +describe('per-command network allow lists', () => { + let loopback: { server: Server; target: string } + let asked: string[] + const denyAndRecord: SandboxAskCallback = async ({ host, port }) => { + asked.push(`${host}:${port}`) + return false + } + + async function start( + network: Partial, + callback?: SandboxAskCallback, + ): Promise { + await SandboxManager.initialize( + { + network: { allowedDomains: [], deniedDomains: [], ...network }, + filesystem: NO_FILESYSTEM_RULES, + }, + callback, + ) + SandboxManager.getSandboxViolationStore().clear() + return SandboxManager.getProxyPort()! + } + + beforeEach(async () => { + await SandboxManager.reset() + loopback = await listenOnLoopback() + asked = [] + }) + + afterEach(async () => { + await SandboxManager.reset() + loopback.server.close() + }) + + it('lifts the default deny for the id the connection presents, and for no other', async () => { + const port = await start({}) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(200) + // Any other id, and no id at all, see the configured lists alone. + expect( + await connectAs(port, loopback.target, usernameFor(randomCommandId())), + ).toBe(403) + expect(await connectAs(port, loopback.target, 'srt')).toBe(403) + expect(violationLines()).toEqual([ + `deny network-outbound ${loopback.target} (host is not on the allow list)`, + `deny network-outbound ${loopback.target} (host is not on the allow list)`, + ]) + }) + + it('applies over SOCKS5 as it does over CONNECT', async () => { + await start({}) + const port = SandboxManager.getSocksProxyPort()! + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + expect( + await socksConnectAs(port, loopback.target, usernameFor(registered)), + ).toBe(0) + expect( + await socksConnectAs( + port, + loopback.target, + usernameFor(randomCommandId()), + ), + ).toBe(2) + }) + + // The one case where the wrap, not usernameFor(), mints the username. + it.if(isMacOS || isLinux)( + 'reaches a sandboxed command through the commandId its wrap was given', + async () => { + await start({}) + const curlAs = async (commandId: string): Promise => { + const wrapped = await SandboxManager.wrapWithSandbox( + // NO_PROXY in the sandbox exempts loopback; this goes to the proxy. + `curl -s --noproxy '' --max-time 3 http://${loopback.target}/`, + undefined, + undefined, + undefined, + { commandId }, + ) + return (await spawnAsync('bash', ['-c', wrapped])).stdout + } + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + expect(await curlAs(registered)).toBe('ok') + expect(await curlAs(randomCommandId())).not.toBe('ok') + expect(violationLines()).toEqual([ + `deny network-outbound ${loopback.target} (host is not on the allow list)`, + ]) + }, + 30000, + ) + + it('is consulted before the ask callback, which still decides every host it does not list', async () => { + const port = await start({}, denyAndRecord) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(200) + expect(asked).toEqual([]) + + expect( + await connectAs(port, 'unlisted.test:443', usernameFor(registered)), + ).toBe(403) + expect(await connectAs(port, loopback.target, 'srt')).toBe(403) + expect(asked).toEqual(['unlisted.test:443', loopback.target]) + }) + + it('uses the grammar of network.allowedDomains: a port suffix narrows an entry, a wildcard widens it', async () => { + const port = await start({}, denyAndRecord) + const registered = randomCommandId() + const [host, listeningPort] = loopback.target.split(':') + // Any valid port but the listener's, which can itself be 65535. + const anotherPort = + Number(listeningPort) === 65535 ? 65534 : Number(listeningPort) + 1 + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [`${host}:${anotherPort}`, '*.wild.test'], + }) + + // Listed for another port only, so the callback is asked and denies. + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + expect(asked).toEqual([loopback.target]) + // The wildcard matches a subdomain (the filter allows; the dial then + // fails, which is not a 403) and, as in the configuration, not the apex. + expect( + await connectAs(port, 'a.wild.test:443', usernameFor(registered)), + ).not.toBe(403) + expect( + await connectAs(port, 'wild.test:443', usernameFor(registered)), + ).toBe(403) + expect(asked).toEqual([loopback.target, 'wild.test:443']) + }) + + it('never overrides a configured deniedDomains entry', async () => { + const port = await start({ deniedDomains: ['127.0.0.1'] }, denyAndRecord) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + expect(violationLines()).toEqual([ + `deny network-outbound ${loopback.target} (host is on the deny list)`, + ]) + expect(asked).toEqual([]) + }) + + it('is ignored entirely under strictAllowlist', async () => { + const port = await start({ strictAllowlist: true }, denyAndRecord) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + expect(violationLines()).toEqual([ + `deny network-outbound ${loopback.target} (host is not on the allow list)`, + ]) + expect(asked).toEqual([]) + }) + + it('never enters network.*, so the configured allow list does not grow', async () => { + await start({ allowedDomains: ['example.com'] }) + SandboxManager.registerCommandNetworkLists(randomCommandId(), { + allowedDomains: [loopback.target, 'percommand.test'], + }) + + expect(SandboxManager.getConfig()?.network.allowedDomains).toEqual([ + 'example.com', + ]) + expect(SandboxManager.getNetworkRestrictionConfig().allowedHosts).toEqual([ + 'example.com', + ]) + }) + + it('stops applying once unregistered; unregistering an id that has no list changes nothing', async () => { + const port = await start({}) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + SandboxManager.unregisterCommandNetworkLists(randomCommandId()) + // Unregistering lets a non-string pass, so a cleanup path never throws. + for (const notAString of [null, undefined, 42]) { + expect(() => + SandboxManager.unregisterCommandNetworkLists( + notAString as unknown as string, + ), + ).not.toThrow() + } + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(200) + + SandboxManager.unregisterCommandNetworkLists(registered) + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + // Twice is as good as once. + SandboxManager.unregisterCommandNetworkLists(registered) + }) + + it('replaces the list when an id is registered again', async () => { + const port = await start({}) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: ['elsewhere.test'], + }) + + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + }) + + it('keeps the list it validated when the caller changes its array afterwards', async () => { + const port = await start({}) + const registered = randomCommandId() + const entries = ['elsewhere.test'] + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: entries, + }) + entries.push(loopback.target) + + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + }) + + it('forgets every list on reset', async () => { + await start({}) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + await SandboxManager.reset() + const port = await start({}) + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + }) + + it('registers under an external network.httpProxyPort, where no list applies, and says so in the debug log', async () => { + const before = process.env.SRT_DEBUG + process.env.SRT_DEBUG = '1' + const warnSpy = spyOn(console, 'warn').mockImplementation(() => {}) + const errorSpy = spyOn(console, 'error').mockImplementation(() => {}) + const warnings = (): string[] => + warnSpy.mock.calls + .map(call => String(call[0])) + .filter(message => message.includes('registerCommandNetworkLists')) + try { + await start({}) + SandboxManager.registerCommandNetworkLists(randomCommandId(), {}) + expect(warnings()).toEqual([]) + + await SandboxManager.reset() + await start({ httpProxyPort: 8888 }) + const registered = randomCommandId() + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + expect(warnings()).toHaveLength(1) + expect(warnings()[0]).toContain('this list will never apply') + expect(warnings()[0]).not.toContain(registered) + // The library's own port still serves SOCKS and CONNECT, and reads no id. + expect( + await connectAs( + SandboxManager.getSocksProxyPort()!, + loopback.target, + usernameFor(registered), + ), + ).toBe(403) + } finally { + warnSpy.mockRestore() + errorSpy.mockRestore() + if (before === undefined) { + delete process.env.SRT_DEBUG + } else { + process.env.SRT_DEBUG = before + } + } + }) + + it('refuses an id shorter than 22 characters, and says what an id has to be', () => { + const lists = { allowedDomains: ['example.com'] } + for (const tooShort of ['', '1', 'inv-0001', 'a'.repeat(21)]) { + expect(() => + SandboxManager.registerCommandNetworkLists(tooShort, lists), + ).toThrow(/at least 22 characters/) + } + expect(() => + SandboxManager.registerCommandNetworkLists('a'.repeat(21), lists), + ).toThrow(/only thing that binds a proxy connection to this allow list/) + expect(() => + SandboxManager.registerCommandNetworkLists('a'.repeat(21), lists), + ).toThrow(/random .* never derived from the command text/) + // A JavaScript caller's null or number is no id either. + for (const notAString of [null, undefined, 1e21]) { + expect(() => + SandboxManager.registerCommandNetworkLists( + notAString as unknown as string, + lists, + ), + ).toThrow(/at least 22 characters/) + } + // The floor itself is accepted, and an empty or absent list is a list. + SandboxManager.registerCommandNetworkLists('a'.repeat(22), lists) + SandboxManager.registerCommandNetworkLists(randomCommandId(), {}) + SandboxManager.registerCommandNetworkLists(randomCommandId(), { + allowedDomains: [], + }) + // So is no second argument at all, which a JavaScript caller can pass. + expect(() => + SandboxManager.registerCommandNetworkLists( + randomCommandId(), + undefined as unknown as { allowedDomains?: string[] }, + ), + ).not.toThrow() + }) + + it('does not repeat the id in the error it throws', () => { + const almost = 'secret-id-0123456789x' + expect(almost).toHaveLength(21) + let message = '' + try { + SandboxManager.registerCommandNetworkLists(almost, {}) + } catch (error) { + message = (error as Error).message + } + expect(message).not.toBe('') + expect(message).not.toContain(almost) + }) + + it('measures the floor on the key it stores, where an unpaired surrogate counts for nothing', () => { + // Each is 22 characters long, and is stored with U+FFFD in place of every + // unpaired surrogate. U+FFFD is not counted. + for (const hollow of [ + '\ud800'.repeat(22), + `${'a'.repeat(21)}\udc00`, + '\ufffd'.repeat(22), + ]) { + expect(hollow).toHaveLength(22) + expect(() => + SandboxManager.registerCommandNetworkLists(hollow, {}), + ).toThrow(/at least 22 characters, unpaired surrogates not counted/) + } + // A pair is a character like any other, counted as String.length does. + SandboxManager.registerCommandNetworkLists('\u{1f600}'.repeat(11), {}) + }) + + // A list arriving at run time cannot carry an entry the configuration + // would have refused, and the other way round. + it.each([ + ['example.com', true], + ['*.example.com', true], + ['api.example.com:443', true], + ['localhost', true], + ['127.0.0.1:3000', true], + ['[::1]', true], + ['[2001:db8::1]:443', true], + ['*', false], + ['*:443', false], + ['*.com', false], + ['https://example.com', false], + ['example.com/path', false], + ['example.com:0', false], + ['example.com:443.evil.test', false], + ['2001:db8::1', false], + ['nodots', false], + ['', false], + ])( + 'accepts the entry %p exactly when network.allowedDomains does (%p)', + (entry, valid) => { + expect( + SandboxRuntimeConfigSchema.safeParse({ + network: { allowedDomains: [entry], deniedDomains: [] }, + filesystem: NO_FILESYSTEM_RULES, + }).success, + ).toBe(valid) + const register = () => + SandboxManager.registerCommandNetworkLists(randomCommandId(), { + allowedDomains: [entry], + }) + if (valid) { + expect(register).not.toThrow() + } else { + expect(register).toThrow(/invalid allowedDomains entry/) + } + }, + ) + + it('refuses an entry that is not a string, and a list that is not an array', () => { + for (const entry of [null, undefined, 443, { host: 'example.com' }]) { + expect(() => + SandboxManager.registerCommandNetworkLists(randomCommandId(), { + allowedDomains: [entry as unknown as string], + }), + ).toThrow(/invalid allowedDomains entry/) + } + expect(() => + SandboxManager.registerCommandNetworkLists(randomCommandId(), { + allowedDomains: 'example.com' as unknown as string[], + }), + ).toThrow(/must be an array/) + }) + + it('registers nothing when any entry is invalid', async () => { + const port = await start({}) + const registered = randomCommandId() + expect(() => + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target, '*.com'], + }), + ).toThrow(/invalid allowedDomains entry "\*\.com"/) + + expect( + await connectAs(port, loopback.target, usernameFor(registered)), + ).toBe(403) + }) + + // An id is normalised before it keys the map, and what the proxy receives + // is decoded rather than compared as bytes, so an id can differ from the + // registered one byte for byte and reach its list all the same. + describe('ids that differ byte for byte from the registered one', () => { + it('share a list when they agree on their first 100 characters', async () => { + const port = await start({}) + const prefix = randomBytes(75).toString('base64url') + expect(prefix).toHaveLength(100) + SandboxManager.registerCommandNetworkLists(`${prefix}-first`, { + allowedDomains: [loopback.target], + }) + + // Both ids mint one username: the key is cut to 100 characters. + expect(usernameFor(`${prefix}-second`)).toBe( + usernameFor(`${prefix}-first`), + ) + expect( + await connectAs(port, loopback.target, usernameFor(`${prefix}-second`)), + ).toBe(200) + + // An uncut username decodes to a longer key, which has no list. + const uncut = Buffer.from(`${prefix}-first`).toString('base64') + expect(await connectAs(port, loopback.target, `srt.${uncut}`)).toBe(403) + + // And unregistering either id removes the one list they share. + SandboxManager.unregisterCommandNetworkLists(`${prefix}-second`) + expect( + await connectAs(port, loopback.target, usernameFor(`${prefix}-first`)), + ).toBe(403) + }) + + it('reach the list through any base64 spelling of the same bytes', async () => { + const port = await start({}) + // 22 characters that encode with padding and with both of the + // characters the URL-safe alphabet spells differently. + const registered = 'id?>id?>id?>id?>id?>~~' + const minted = encodeSandboxedCommand(registered) + expect(minted).toMatch(/[+/]/) + expect(minted).toMatch(/=$/) + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + const unpadded = minted.replace(/=+$/, '') + const urlSafe = unpadded.replace(/\+/g, '-').replace(/\//g, '_') + for (const spelling of [minted, unpadded, urlSafe]) { + expect(await connectAs(port, loopback.target, `srt.${spelling}`)).toBe( + 200, + ) + } + }) + + it('share a list when they differ only in an unpaired surrogate', async () => { + const port = await start({}) + const stem = randomCommandId() + // Neither half of a surrogate pair can be encoded on its own; each + // becomes U+FFFD, so both ids normalise to one key. + SandboxManager.registerCommandNetworkLists(`${stem}\ud800`, { + allowedDomains: [loopback.target], + }) + + expect( + await connectAs(port, loopback.target, usernameFor(`${stem}\udfff`)), + ).toBe(200) + expect( + await connectAs(port, loopback.target, usernameFor(`${stem}\ufffd`)), + ).toBe(200) + // The stem alone is a different key. + expect(await connectAs(port, loopback.target, usernameFor(stem))).toBe( + 403, + ) + }) + + it('do not reach it by differing in case', async () => { + const port = await start({}) + const registered = `${randomCommandId()}abc` + SandboxManager.registerCommandNetworkLists(registered, { + allowedDomains: [loopback.target], + }) + + expect( + await connectAs( + port, + loopback.target, + usernameFor(registered.toUpperCase()), + ), + ).toBe(403) + }) + }) +}) + +describe('ask callback answers', () => { + let loopback: { server: Server; target: string } + + /** One CONNECT to the loopback listener, decided by `answer` alone. */ + async function decideWith( + answer: unknown, + ): Promise<{ status: number | undefined; lines: string[] }> { + await SandboxManager.initialize( + { + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: NO_FILESYSTEM_RULES, + }, + (async () => answer) as SandboxAskCallback, + ) + SandboxManager.getSandboxViolationStore().clear() + const status = await connectAs( + SandboxManager.getProxyPort()!, + loopback.target, + 'srt', + ) + return { status, lines: violationLines() } + } + + const deniedBecause = (reason: string): string[] => [ + `deny network-outbound ${loopback.target} (${reason})`, + ] + + beforeEach(async () => { + await SandboxManager.reset() + loopback = await listenOnLoopback() + }) + + afterEach(async () => { + await SandboxManager.reset() + loopback.server.close() + }) + + it('declares that it reads a denial with a reason', () => { + expect(SandboxManager.askCallbackDenyReason).toBe(true) + }) + + it('allows on true', async () => { + expect(await decideWith(true)).toEqual({ status: 200, lines: [] }) + }) + + it('denies on false, with the generic reason', async () => { + expect(await decideWith(false)).toEqual({ + status: 403, + lines: deniedBecause('user denied'), + }) + }) + + it('denies on { allow: false, reason }, and reports that reason', async () => { + const callback: SandboxAskCallback = async () => ({ + allow: false, + reason: 'not listed for this command; list the host and run it again', + }) + expect(await decideWith(await callback({ host: '', port: 0 }))).toEqual({ + status: 403, + lines: deniedBecause( + 'not listed for this command; list the host and run it again', + ), + }) + }) + + it.each([ + ['an empty reason', { allow: false, reason: '' }], + ['a reason of nothing but whitespace', { allow: false, reason: ' \n\t ' }], + ['no reason', { allow: false }], + ['a reason that is not a string', { allow: false, reason: 42 }], + ])('denies with the generic reason on %s', async (_name, answer) => { + expect(await decideWith(answer)).toEqual({ + status: 403, + lines: deniedBecause('user denied'), + }) + }) + + // A filter that tested truthiness would allow every one of these. + it.each([ + ['the number 1', 1], + ['a non-empty string', 'yes'], + ['an empty object', {}], + ['an empty array', []], + ['an object that says allow: true', { allow: true }], + ['the string "true"', 'true'], + // A reason is reported only for an answer that says allow: false. + [ + 'an object that says allow: true and gives a reason', + { allow: true, reason: 'approved for this session' }, + ], + [ + 'an object that gives a reason and no allow', + { reason: 'no verdict given' }, + ], + [ + 'an object whose allow is falsy but not false', + { allow: 0, reason: 'zero is not false' }, + ], + ])( + 'denies on a truthy answer that is not true: %s', + async (_name, answer) => { + expect(await decideWith(answer)).toEqual({ + status: 403, + lines: deniedBecause('user denied'), + }) + }, + ) + + it.each([ + ['undefined', undefined], + ['null', null], + ['zero', 0], + ])( + 'denies on a falsy answer that is not false: %s', + async (_name, answer) => { + expect(await decideWith(answer)).toEqual({ + status: 403, + lines: deniedBecause('user denied'), + }) + }, + ) + + it('cuts an over-long reason to 500 characters', async () => { + const { status, lines } = await decideWith({ + allow: false, + reason: 'x'.repeat(2000), + }) + expect(status).toBe(403) + expect(lines).toEqual(deniedBecause('x'.repeat(500))) + }) + + it('cuts between characters, never through a surrogate pair', async () => { + // The pair occupies code units 499 and 500, so a cut at 500 lands inside. + const { lines } = await decideWith({ + allow: false, + reason: `${'x'.repeat(499)}\u{1f600}tail`, + }) + expect(lines).toEqual(deniedBecause('x'.repeat(499))) + }) + + it('counts the 500 in UTF-16 code units, so a character outside the BMP counts twice', async () => { + const { lines } = await decideWith({ + allow: false, + reason: '\u{1f600}'.repeat(500), + }) + expect(lines).toEqual(deniedBecause('\u{1f600}'.repeat(250))) + }) + + it('leaves no space at the end when the cut lands just after one', async () => { + // Code unit 500 is the space. Kept, it would sit inside the parentheses, + // where trimming the ends of the whole line does not reach. + const { lines } = await decideWith({ + allow: false, + reason: `${'x'.repeat(499)} tail`, + }) + expect(lines).toEqual(deniedBecause('x'.repeat(499))) + }) + + it('strips control characters from the reason, and what would close the violations envelope', async () => { + const { lines } = await decideWith({ + allow: false, + reason: + 'first line\nsecond\x00line\x1b[31m red\u009b\u202eflipped\u200b', + }) + expect(lines).toHaveLength(1) + // eslint-disable-next-line no-control-regex + expect(lines[0]).not.toMatch(/[\x00-\x1f\x7f-\x9f\u202e\u200b<>]/) + expect(lines).toEqual( + deniedBecause( + 'first line second line [31m red flipped /sandbox_violations', + ), + ) + }) + + it('removes angle brackets outright, and turns a tab or a run of joiners into one space', async () => { + const { lines } = await decideWith({ + allow: false, + reason: 'a\tb: re-run with listed x\u200d\u200dy', + }) + expect(lines).toEqual(deniedBecause('a b: re-run with host listed x y')) + }) + + it('counts the cut in what is displayed, not in what was stripped', async () => { + // 600 control characters collapse to one space, so `after` is not cut. + const { lines } = await decideWith({ + allow: false, + reason: `before${'\x07'.repeat(600)}after`, + }) + expect(lines).toEqual(deniedBecause('before after')) + }) + + it('denies with "permission prompt failed" when the callback throws', async () => { + await SandboxManager.initialize( + { + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: NO_FILESYSTEM_RULES, + }, + async () => { + throw new Error('prompt went away') + }, + ) + SandboxManager.getSandboxViolationStore().clear() + expect( + await connectAs(SandboxManager.getProxyPort()!, loopback.target, 'srt'), + ).toBe(403) + expect(violationLines()).toEqual(deniedBecause('permission prompt failed')) + }) + + // The debug log tells an embedder why a truthy answer did not allow. + it('warns in the debug log about an answer that is neither a boolean nor { allow: false }, and about no other', async () => { + const before = process.env.SRT_DEBUG + process.env.SRT_DEBUG = '1' + const warnSpy = spyOn(console, 'warn').mockImplementation(() => {}) + const errorSpy = spyOn(console, 'error').mockImplementation(() => {}) + const warningsAfter = async (answer: unknown): Promise => { + await SandboxManager.reset() + warnSpy.mockClear() + await decideWith(answer) + return warnSpy.mock.calls + .map(call => String(call[0])) + .filter(message => message.includes('Ask callback answered neither')) + } + try { + for (const undeclared of [1, 'yes', {}, { allow: true, reason: 'ok' }]) { + const warnings = await warningsAfter(undeclared) + expect(warnings).toHaveLength(1) + expect(warnings[0]).toContain('treating it as a denial') + expect(warnings[0]).toContain(loopback.target) + } + for (const declared of [true, false, { allow: false, reason: 'no' }]) { + expect(await warningsAfter(declared)).toEqual([]) + } + } finally { + warnSpy.mockRestore() + errorSpy.mockRestore() + if (before === undefined) { + delete process.env.SRT_DEBUG + } else { + process.env.SRT_DEBUG = before + } + } + }) +}) + describe('SandboxManager.updateConfig', () => { beforeEach(async () => { await SandboxManager.reset()