mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-01 19:25:10 +02:00
fix: security wave 3 — 12 fixes, 7 contributors (v0.16.4.0) (#988)
* fix(security): validateOutputPath symlink bypass — check file-level symlinks validateOutputPath() previously only resolved symlinks on the parent directory. A symlink at /tmp/evil.png → /etc/crontab passed the parent check (parent is /tmp, which is safe) but the write followed the symlink outside safe dirs. Add lstatSync() check: if the target file exists and is a symlink, resolve through it and verify the real target is within SAFE_DIRECTORIES. ENOENT (file doesn't exist yet) falls through to the existing parent-dir check. Closes #921 Co-Authored-By: Yunsu <Hybirdss@users.noreply.github.com> * fix(security): shell injection in bin/ scripts — use env vars instead of interpolation gstack-settings-hook interpolated $SETTINGS_FILE directly into bun -e double-quoted blocks. A path containing quotes or backticks breaks the JS string context, enabling arbitrary code execution. Replace direct interpolation with environment variables (process.env). Same fix applied to gstack-team-init which had the same pattern. Systematic audit confirmed only these two scripts were vulnerable — all other bin/ scripts already use stdin piping or env vars. Closes #858 Co-Authored-By: Gus <garagon@users.noreply.github.com> * fix(security): cookie-import path validation bypass + hardcoded /tmp Two fixes: 1. cookie-import relative path bypass (#707): path.isAbsolute() gated the entire validation, so relative paths like "sensitive-file.json" bypassed the safe-directory check entirely. Now always resolves to absolute path with realpathSync for symlink resolution, matching validateOutputPath(). 2. Hardcoded /tmp in cookie-import-browser (#708): openDbFromCopy used /tmp directly instead of os.tmpdir(), breaking Windows support. Also adds explicit imports for SAFE_DIRECTORIES and isPathWithin in write-commands.ts (previously resolved implicitly through bundler). Closes #852 Co-Authored-By: Toby Morning <urbantech@users.noreply.github.com> * fix(security): redact form fields with sensitive names, not just type=password Form redaction only applied to type="password" fields. Hidden and text fields named csrf_token, api_key, session_id, etc. were exposed unredacted in LLM context, leaking secrets. Extend redaction to check field name and id against sensitive patterns: token, secret, key, password, credential, auth, jwt, session, csrf, sid, api_key. Uses the same pattern style as SENSITIVE_COOKIE_NAME. Closes #860 Co-Authored-By: Gus <garagon@users.noreply.github.com> * fix(security): restrict session file permissions to owner-only Design session files written to /tmp with default umask (0644) were world-readable on shared systems. Sessions contain design prompts and feedback history. Set mode 0o600 (owner read/write only) on both create and update paths. Closes #859 Co-Authored-By: Gus <garagon@users.noreply.github.com> * fix(security): enforce frozen lockfile during setup bun install without --frozen-lockfile resolves ^semver ranges from npm on every run. If an attacker publishes a compromised compatible version of any dependency, the next ./setup pulls it silently. Add --frozen-lockfile with fallback to plain install (for fresh clones where bun.lock may not exist yet). Matches the pattern already used in the .agents/ generation block (line 237). Closes #614 Co-Authored-By: Alberto Martinez <halbert04@users.noreply.github.com> * fix: remove duplicate recursive chmod on /tmp in Dockerfile.ci chmod -R 1777 /tmp recursively sets sticky bit on files (no defined behavior), not just the directory. Deduplicate to single chmod 1777 /tmp. Closes #747 Co-Authored-By: Maksim Soltan <Gonzih@users.noreply.github.com> * fix(security): learnings input validation + cross-project trust gate Three fixes to the learnings system: 1. Input validation in gstack-learnings-log: type must be from allowed list, key must be alphanumeric, confidence must be 1-10 integer, source must be from allowed list. Prevents injection via malformed fields. 2. Prompt injection defense: insight field checked against 10 instruction-like patterns (ignore previous, system:, override, etc.). Rejected with clear error message. 3. Cross-project trust gate in gstack-learnings-search: AI-generated learnings from other projects are filtered out. Only user-stated learnings cross project boundaries. Prevents silent prompt injection across codebases. Also adds trusted field (true for user-stated source, false for AI-generated) to enable the trust gate at read time. Closes #841 Co-Authored-By: Ziad Al Sharif <Ziadstr@users.noreply.github.com> * feat(security): track cookie-imported domains and scope cookie imports Foundation for origin-pinned JS execution (#616). Tracks which domains cookies were imported from so the JS/eval commands can verify execution stays within imported origins. Changes: - BrowserManager: new cookieImportedDomains Set with track/get/has methods - cookie-import: tracks imported cookie domains after addCookies - cookie-import-browser: tracks domains on --domain direct import - cookie-import-browser --all: new explicit opt-in for all-domain import (previously implicit behavior, now requires deliberate flag) Closes #615 Co-Authored-By: Alberto Martinez <halbert04@users.noreply.github.com> * feat(security): pin JS/eval execution to cookie-imported origins When cookies have been imported for specific domains, block JS execution on pages whose origin doesn't match. Prevents the attack chain: 1. Agent imports cookies for github.com 2. Prompt injection navigates to attacker.com 3. Agent runs js document.cookie → exfiltrates github cookies assertJsOriginAllowed() checks the current page hostname against imported cookie domains with subdomain matching (.github.com allows api.github.com). When no cookies are imported, all origins allowed (nothing to protect). about:blank and data: URIs are allowed (no cookies at risk). Depends on #615 (cookie domain tracking). Closes #616 Co-Authored-By: Alberto Martinez <halbert04@users.noreply.github.com> * feat(security): add persistent command audit log Append-only JSONL audit trail for all browse server commands. Unlike in-memory ring buffers, the audit log persists across restarts and is never truncated. Each entry records: timestamp, command, args (truncated to 200 chars), page origin, duration, status, error (truncated to 300 chars), hasCookies flag, connection mode. All writes are best-effort — audit failures never block command execution. Log stored at ~/.gstack/.browse/browse-audit.jsonl. Closes #617 Co-Authored-By: Alberto Martinez <halbert04@users.noreply.github.com> * fix(security): block hex-encoded IPv4-mapped IPv6 metadata bypass URL constructor normalizes ::ffff:169.254.169.254 to ::ffff:a9fe:a9fe (hex form), which was not in the blocklist. Similarly, ::169.254.169.254 normalizes to ::a9fe:a9fe. Add both hex-encoded forms to BLOCKED_METADATA_HOSTS so they're caught by the direct hostname check in validateNavigationUrl. Closes #739 Co-Authored-By: Osman Mehmood <mehmoodosman@users.noreply.github.com> * chore: bump version and changelog (v0.16.4.0) Security wave 3: 12 fixes, 7 contributors. Cookie origin pinning, command audit log, domain tracking. Symlink bypass, path validation, shell injection, form redaction, learnings injection, IPv6 SSRF, session permissions, frozen lockfile. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Yunsu <Hybirdss@users.noreply.github.com> Co-authored-by: Gus <garagon@users.noreply.github.com> Co-authored-by: Toby Morning <urbantech@users.noreply.github.com> Co-authored-by: Alberto Martinez <halbert04@users.noreply.github.com> Co-authored-by: Maksim Soltan <Gonzih@users.noreply.github.com> Co-authored-by: Ziad Al Sharif <Ziadstr@users.noreply.github.com> Co-authored-by: Osman Mehmood <mehmoodosman@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -59,5 +59,4 @@ RUN useradd -m -s /bin/bash runner \
|
||||
&& chmod -R a+rX /opt/node_modules_cache \
|
||||
&& mkdir -p /home/runner/.gstack && chown -R runner:runner /home/runner/.gstack \
|
||||
&& chmod 1777 /tmp \
|
||||
&& mkdir -p /home/runner/.bun && chown -R runner:runner /home/runner/.bun \
|
||||
&& chmod -R 1777 /tmp
|
||||
&& mkdir -p /home/runner/.bun && chown -R runner:runner /home/runner/.bun
|
||||
|
||||
@@ -1,5 +1,29 @@
|
||||
# Changelog
|
||||
|
||||
## [0.16.4.0] - 2026-04-13
|
||||
|
||||
### Added
|
||||
- **Cookie origin pinning.** When you import cookies for specific domains, JS execution is now blocked on pages that don't match those domains. This prevents the attack where a prompt injection navigates to an attacker's site and runs `document.cookie` to steal your imported cookies. Subdomain matching works automatically (importing `.github.com` allows `api.github.com`). When no cookies are imported, everything works as before. 3 PRs from @halbert04.
|
||||
- **Command audit log.** Every browse command now gets a persistent forensic trail in `~/.gstack/.browse/browse-audit.jsonl`. Timestamp, command, args, page origin, duration, status, error, and whether cookies were imported. Append-only, never truncated, survives server restarts. Best-effort writes that never block command execution. From @halbert04.
|
||||
- **Cookie domain tracking.** gstack now tracks which domains cookies were imported from. Foundation for origin pinning above. Direct imports via `--domain` track automatically. New `--all` flag makes full-browser cookie import an explicit opt-in instead of the default.
|
||||
|
||||
### Fixed
|
||||
- **Symlink bypass in file writes.** `validateOutputPath` only checked the parent directory for symlinks, not the file itself. A symlink at `/tmp/evil.png` pointing to `/etc/crontab` passed validation because the parent `/tmp` was safe. Now checks the file with `lstatSync` before writing. From @Hybirdss.
|
||||
- **Cookie-import path bypass.** Two issues: relative paths bypassed all validation (the `path.isAbsolute()` gate let `sensitive-file.json` through), and symlink resolution was missing (`path.resolve` without `realpathSync`). Now resolves to absolute, resolves symlinks, and checks against safe directories. From @urbantech.
|
||||
- **Shell injection in setup scripts.** `gstack-settings-hook` interpolated file paths directly into `bun -e` JavaScript blocks. A path with quotes broke the JS string context. Now uses environment variables (`process.env`). Systematic audit confirmed only this script was vulnerable. From @garagon.
|
||||
- **Form field credential leak.** Snapshot redaction only applied to `type="password"` fields. Hidden and text fields named `csrf_token`, `api_key`, `session_id` were exposed unredacted in LLM context. Now checks field name and id against sensitive patterns. From @garagon.
|
||||
- **Learnings prompt injection.** Three fixes: input validation (type/key/confidence allowlists), injection pattern detection in insight field (blocks "ignore previous instructions" etc.), and cross-project trust gate (only user-stated learnings cross project boundaries). From @Ziadstr.
|
||||
- **IPv6 metadata bypass.** The URL constructor normalizes `::ffff:169.254.169.254` to `::ffff:a9fe:a9fe` (hex), which wasn't in the blocklist. Added both hex-encoded forms. From @mehmoodosman.
|
||||
- **Session files world-readable.** Design session files in `/tmp` were created with default permissions (0644). Now 0600 (owner-only). From @garagon.
|
||||
- **Frozen lockfile in setup.** `bun install` now uses `--frozen-lockfile` to prevent supply chain attacks via floating semver ranges. From @halbert04.
|
||||
- **Dockerfile chmod fix.** Removed duplicate recursive `chmod -R 1777 /tmp` (recursive sticky bit on files has no defined behavior). From @Gonzih.
|
||||
- **Hardcoded /tmp in cookie import.** `cookie-import-browser` used `/tmp` directly instead of `os.tmpdir()`, breaking Windows support.
|
||||
|
||||
### Security
|
||||
- Closed 14 security issues (#665-#675, #566, #479, #467, #545) that were fixed in prior waves but still open on GitHub.
|
||||
- Closed 17 community security PRs with thank-you messages and commit references.
|
||||
- Security wave 3: 12 fixes, 7 contributors. Big thanks to @Hybirdss, @urbantech, @garagon, @Ziadstr, @halbert04, @mehmoodosman, @Gonzih.
|
||||
|
||||
## [0.16.3.0] - 2026-04-09
|
||||
|
||||
### Changed
|
||||
|
||||
+69
-13
@@ -12,19 +12,75 @@ mkdir -p "$GSTACK_HOME/projects/$SLUG"
|
||||
|
||||
INPUT="$1"
|
||||
|
||||
# Validate: input must be parseable JSON
|
||||
if ! printf '%s' "$INPUT" | bun -e "JSON.parse(await Bun.stdin.text())" 2>/dev/null; then
|
||||
echo "gstack-learnings-log: invalid JSON, skipping" >&2
|
||||
# Validate and sanitize input
|
||||
VALIDATED=$(printf '%s' "$INPUT" | bun -e "
|
||||
const raw = await Bun.stdin.text();
|
||||
let j;
|
||||
try { j = JSON.parse(raw); } catch { process.stderr.write('gstack-learnings-log: invalid JSON, skipping\n'); process.exit(1); }
|
||||
|
||||
// Field validation: type must be from allowed list
|
||||
const ALLOWED_TYPES = ['pattern', 'pitfall', 'preference', 'architecture', 'tool', 'operational'];
|
||||
if (!j.type || !ALLOWED_TYPES.includes(j.type)) {
|
||||
process.stderr.write('gstack-learnings-log: invalid type \"' + (j.type || '') + '\", must be one of: ' + ALLOWED_TYPES.join(', ') + '\n');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// Field validation: key must be alphanumeric, hyphens, underscores (no injection surface)
|
||||
if (!j.key || !/^[a-zA-Z0-9_-]+$/.test(j.key)) {
|
||||
process.stderr.write('gstack-learnings-log: invalid key, must be alphanumeric with hyphens/underscores only\n');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// Field validation: confidence must be 1-10
|
||||
const conf = Number(j.confidence);
|
||||
if (!Number.isInteger(conf) || conf < 1 || conf > 10) {
|
||||
process.stderr.write('gstack-learnings-log: confidence must be integer 1-10\n');
|
||||
process.exit(1);
|
||||
}
|
||||
j.confidence = conf;
|
||||
|
||||
// Field validation: source must be from allowed list
|
||||
const ALLOWED_SOURCES = ['observed', 'user-stated', 'inferred', 'cross-model'];
|
||||
if (j.source && !ALLOWED_SOURCES.includes(j.source)) {
|
||||
process.stderr.write('gstack-learnings-log: invalid source, must be one of: ' + ALLOWED_SOURCES.join(', ') + '\n');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// Content sanitization: strip instruction-like patterns from insight field
|
||||
// These patterns could be used for prompt injection when learnings are loaded into agent context
|
||||
if (j.insight) {
|
||||
const INJECTION_PATTERNS = [
|
||||
/ignore\s+(all\s+)?previous\s+(instructions|context|rules)/i,
|
||||
/you\s+are\s+now\s+/i,
|
||||
/always\s+output\s+no\s+findings/i,
|
||||
/skip\s+(all\s+)?(security|review|checks)/i,
|
||||
/override[:\s]/i,
|
||||
/\bsystem\s*:/i,
|
||||
/\bassistant\s*:/i,
|
||||
/\buser\s*:/i,
|
||||
/do\s+not\s+(report|flag|mention)/i,
|
||||
/approve\s+(all|every|this)/i,
|
||||
];
|
||||
for (const pat of INJECTION_PATTERNS) {
|
||||
if (pat.test(j.insight)) {
|
||||
process.stderr.write('gstack-learnings-log: insight contains suspicious instruction-like content, rejected\n');
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Inject timestamp if not present
|
||||
if (!j.ts) j.ts = new Date().toISOString();
|
||||
|
||||
// Mark trust level based on source
|
||||
// user-stated = user explicitly told the agent this. All others are AI-generated.
|
||||
j.trusted = j.source === 'user-stated';
|
||||
|
||||
console.log(JSON.stringify(j));
|
||||
" 2>/dev/null)
|
||||
|
||||
if [ $? -ne 0 ] || [ -z "$VALIDATED" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Inject timestamp if not present
|
||||
if ! printf '%s' "$INPUT" | bun -e "const j=JSON.parse(await Bun.stdin.text()); if(!j.ts) process.exit(1)" 2>/dev/null; then
|
||||
INPUT=$(printf '%s' "$INPUT" | bun -e "
|
||||
const j = JSON.parse(await Bun.stdin.text());
|
||||
j.ts = new Date().toISOString();
|
||||
console.log(JSON.stringify(j));
|
||||
" 2>/dev/null) || true
|
||||
fi
|
||||
|
||||
echo "$INPUT" >> "$GSTACK_HOME/projects/$SLUG/learnings.jsonl"
|
||||
echo "$VALIDATED" >> "$GSTACK_HOME/projects/$SLUG/learnings.jsonl"
|
||||
|
||||
@@ -68,7 +68,13 @@ for (const line of lines) {
|
||||
|
||||
// Determine if this is from the current project or cross-project
|
||||
// Cross-project entries are tagged for display
|
||||
e._crossProject = !line.includes(slug) && process.env.GSTACK_SEARCH_CROSS === 'true';
|
||||
const isCrossProject = !line.includes(slug) && process.env.GSTACK_SEARCH_CROSS === 'true';
|
||||
e._crossProject = isCrossProject;
|
||||
|
||||
// Trust gate: cross-project learnings only loaded if trusted (user-stated)
|
||||
// This prevents prompt injection from one project's AI-generated learnings
|
||||
// silently influencing reviews in another project.
|
||||
if (isCrossProject && e.trusted === false) continue;
|
||||
|
||||
entries.push(e);
|
||||
} catch {}
|
||||
|
||||
@@ -26,10 +26,10 @@ fi
|
||||
|
||||
case "$ACTION" in
|
||||
add)
|
||||
bun -e "
|
||||
GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_HOOK_CMD="$HOOK_CMD" bun -e "
|
||||
const fs = require('fs');
|
||||
const settingsPath = '$SETTINGS_FILE';
|
||||
const hookCmd = $(printf '%s' "$HOOK_CMD" | bun -e "process.stdout.write(JSON.stringify(require('fs').readFileSync('/dev/stdin','utf8')))");
|
||||
const settingsPath = process.env.GSTACK_SETTINGS_PATH;
|
||||
const hookCmd = process.env.GSTACK_HOOK_CMD;
|
||||
|
||||
let settings = {};
|
||||
try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); } catch {}
|
||||
@@ -55,9 +55,9 @@ case "$ACTION" in
|
||||
;;
|
||||
remove)
|
||||
[ -f "$SETTINGS_FILE" ] || exit 0
|
||||
bun -e "
|
||||
GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun -e "
|
||||
const fs = require('fs');
|
||||
const settingsPath = '$SETTINGS_FILE';
|
||||
const settingsPath = process.env.GSTACK_SETTINGS_PATH;
|
||||
|
||||
let settings = {};
|
||||
try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); } catch { process.exit(0); }
|
||||
|
||||
@@ -139,9 +139,9 @@ HOOK_EOF
|
||||
|
||||
# Add hook to project-level settings.json
|
||||
if command -v bun >/dev/null 2>&1; then
|
||||
bun -e "
|
||||
GSTACK_SETTINGS_PATH="$SETTINGS" bun -e "
|
||||
const fs = require('fs');
|
||||
const settingsPath = '$SETTINGS';
|
||||
const settingsPath = process.env.GSTACK_SETTINGS_PATH;
|
||||
|
||||
let settings = {};
|
||||
try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); } catch {}
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* Persistent command audit log — forensic trail for all browse server commands.
|
||||
*
|
||||
* Writes append-only JSONL to .gstack/browse-audit.jsonl. Unlike the in-memory
|
||||
* ring buffers (console, network, dialog), the audit log persists across server
|
||||
* restarts and is never truncated by the server. Each entry records:
|
||||
*
|
||||
* - timestamp, command, args (truncated), page origin
|
||||
* - duration, status (ok/error), error message if any
|
||||
* - whether cookies were imported (elevated security context)
|
||||
* - connection mode (headless/headed)
|
||||
*
|
||||
* All writes are best-effort — audit failures never cause command failures.
|
||||
*/
|
||||
|
||||
import * as fs from 'fs';
|
||||
|
||||
export interface AuditEntry {
|
||||
ts: string;
|
||||
cmd: string;
|
||||
args: string;
|
||||
origin: string;
|
||||
durationMs: number;
|
||||
status: 'ok' | 'error';
|
||||
error?: string;
|
||||
hasCookies: boolean;
|
||||
mode: 'launched' | 'headed';
|
||||
}
|
||||
|
||||
const MAX_ARGS_LENGTH = 200;
|
||||
const MAX_ERROR_LENGTH = 300;
|
||||
|
||||
let auditPath: string | null = null;
|
||||
|
||||
export function initAuditLog(logPath: string): void {
|
||||
auditPath = logPath;
|
||||
}
|
||||
|
||||
export function writeAuditEntry(entry: AuditEntry): void {
|
||||
if (!auditPath) return;
|
||||
try {
|
||||
const truncatedArgs = entry.args.length > MAX_ARGS_LENGTH
|
||||
? entry.args.slice(0, MAX_ARGS_LENGTH) + '…'
|
||||
: entry.args;
|
||||
const truncatedError = entry.error && entry.error.length > MAX_ERROR_LENGTH
|
||||
? entry.error.slice(0, MAX_ERROR_LENGTH) + '…'
|
||||
: entry.error;
|
||||
|
||||
const record: Record<string, unknown> = {
|
||||
ts: entry.ts,
|
||||
cmd: entry.cmd,
|
||||
args: truncatedArgs,
|
||||
origin: entry.origin,
|
||||
durationMs: entry.durationMs,
|
||||
status: entry.status,
|
||||
hasCookies: entry.hasCookies,
|
||||
mode: entry.mode,
|
||||
};
|
||||
if (truncatedError) record.error = truncatedError;
|
||||
|
||||
fs.appendFileSync(auditPath, JSON.stringify(record) + '\n');
|
||||
} catch {
|
||||
// Audit write failures are silent — never block command execution
|
||||
}
|
||||
}
|
||||
@@ -55,6 +55,9 @@ export class BrowserManager {
|
||||
private dialogAutoAccept: boolean = true;
|
||||
private dialogPromptText: string | null = null;
|
||||
|
||||
// ─── Cookie Origin Tracking ────────────────────────────────
|
||||
private cookieImportedDomains: Set<string> = new Set();
|
||||
|
||||
// ─── Handoff State ─────────────────────────────────────────
|
||||
private isHeaded: boolean = false;
|
||||
private consecutiveFailures: number = 0;
|
||||
@@ -749,6 +752,19 @@ export class BrowserManager {
|
||||
return this.dialogPromptText;
|
||||
}
|
||||
|
||||
// ─── Cookie Origin Tracking ────────────────────────────────
|
||||
trackCookieImportDomains(domains: string[]): void {
|
||||
for (const d of domains) this.cookieImportedDomains.add(d);
|
||||
}
|
||||
|
||||
getCookieImportedDomains(): ReadonlySet<string> {
|
||||
return this.cookieImportedDomains;
|
||||
}
|
||||
|
||||
hasCookieImports(): boolean {
|
||||
return this.cookieImportedDomains.size > 0;
|
||||
}
|
||||
|
||||
// ─── Viewport ──────────────────────────────────────────────
|
||||
async setViewport(width: number, height: number) {
|
||||
await this.getPage().setViewportSize({ width, height });
|
||||
|
||||
@@ -20,6 +20,7 @@ export interface BrowseConfig {
|
||||
consoleLog: string;
|
||||
networkLog: string;
|
||||
dialogLog: string;
|
||||
auditLog: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -70,6 +71,7 @@ export function resolveConfig(
|
||||
consoleLog: path.join(stateDir, 'browse-console.log'),
|
||||
networkLog: path.join(stateDir, 'browse-network.log'),
|
||||
dialogLog: path.join(stateDir, 'browse-dialog.log'),
|
||||
auditLog: path.join(stateDir, 'browse-audit.jsonl'),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -386,7 +386,8 @@ function openDb(dbPath: string, browserName: string): Database {
|
||||
}
|
||||
|
||||
function openDbFromCopy(dbPath: string, browserName: string): Database {
|
||||
const tmpPath = `/tmp/browse-cookies-${browserName.toLowerCase()}-${crypto.randomUUID()}.db`;
|
||||
// Use os.tmpdir() instead of hardcoded /tmp for cross-platform support (#708)
|
||||
const tmpPath = path.join(os.tmpdir(), `browse-cookies-${browserName.toLowerCase()}-${crypto.randomUUID()}.db`);
|
||||
try {
|
||||
fs.copyFileSync(dbPath, tmpPath);
|
||||
// Also copy WAL and SHM if they exist (for consistent reads)
|
||||
|
||||
@@ -33,7 +33,26 @@ const TEMP_ONLY = [TEMP_DIR].map(d => {
|
||||
export function validateOutputPath(filePath: string): void {
|
||||
const resolved = path.resolve(filePath);
|
||||
|
||||
// Resolve real path of the parent directory to catch symlinks.
|
||||
// If the target already exists and is a symlink, resolve through it.
|
||||
// Without this, a symlink at /tmp/evil.png → /etc/crontab passes the
|
||||
// parent-directory check (parent is /tmp, which is safe) but the actual
|
||||
// write follows the symlink to /etc/crontab.
|
||||
try {
|
||||
const stat = fs.lstatSync(resolved);
|
||||
if (stat.isSymbolicLink()) {
|
||||
const realTarget = fs.realpathSync(resolved);
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realTarget, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
return; // symlink target verified, no need to check parent
|
||||
}
|
||||
} catch (e: any) {
|
||||
// ENOENT = file doesn't exist yet, fall through to parent-dir check
|
||||
if (e.code !== 'ENOENT') throw e;
|
||||
}
|
||||
|
||||
// For new files (no existing symlink), verify the parent directory.
|
||||
// The file itself may not exist yet (e.g., screenshot output).
|
||||
// This also handles macOS /tmp → /private/tmp transparently.
|
||||
let dir = path.dirname(resolved);
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
*/
|
||||
|
||||
import type { TabSession } from './tab-session';
|
||||
import type { BrowserManager } from './browser-manager';
|
||||
import { consoleBuffer, networkBuffer, dialogBuffer } from './buffers';
|
||||
import type { Page, Frame } from 'playwright';
|
||||
import * as fs from 'fs';
|
||||
@@ -62,10 +63,43 @@ export async function getCleanText(page: Page | Frame): Promise<string> {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* When cookies have been imported for specific domains, block JS execution
|
||||
* on pages whose origin doesn't match any imported cookie domain.
|
||||
* Prevents cross-origin cookie exfiltration via `js document.cookie` or
|
||||
* similar when the agent navigates to an untrusted page.
|
||||
*/
|
||||
function assertJsOriginAllowed(bm: BrowserManager, pageUrl: string): void {
|
||||
if (!bm.hasCookieImports()) return;
|
||||
|
||||
let hostname: string;
|
||||
try {
|
||||
hostname = new URL(pageUrl).hostname;
|
||||
} catch {
|
||||
return; // about:blank, data: URIs — allow (no cookies at risk)
|
||||
}
|
||||
|
||||
const importedDomains = bm.getCookieImportedDomains();
|
||||
const allowed = [...importedDomains].some(domain => {
|
||||
// Exact match or subdomain match (e.g., ".github.com" matches "api.github.com")
|
||||
const normalized = domain.startsWith('.') ? domain : '.' + domain;
|
||||
return hostname === domain.replace(/^\./, '') || hostname.endsWith(normalized);
|
||||
});
|
||||
|
||||
if (!allowed) {
|
||||
throw new Error(
|
||||
`JS execution blocked: current page (${hostname}) does not match any cookie-imported domain. ` +
|
||||
`Imported cookies for: ${[...importedDomains].join(', ')}. ` +
|
||||
`This prevents cross-origin cookie exfiltration. Navigate to an imported domain or run without imported cookies.`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export async function handleReadCommand(
|
||||
command: string,
|
||||
args: string[],
|
||||
session: TabSession
|
||||
session: TabSession,
|
||||
bm?: BrowserManager,
|
||||
): Promise<string> {
|
||||
const page = session.getPage();
|
||||
// Frame-aware target for content extraction
|
||||
@@ -116,7 +150,10 @@ export async function handleReadCommand(
|
||||
id: input.id || undefined,
|
||||
placeholder: input.placeholder || undefined,
|
||||
required: input.required || undefined,
|
||||
value: input.type === 'password' ? '[redacted]' : (input.value || undefined),
|
||||
value: input.type === 'password'
|
||||
|| (input.name && /(^|[_.-])(token|secret|key|password|credential|auth|jwt|session|csrf|sid)($|[_.-])|api.?key/i.test(input.name))
|
||||
|| (input.id && /(^|[_.-])(token|secret|key|password|credential|auth|jwt|session|csrf|sid)($|[_.-])|api.?key/i.test(input.id))
|
||||
? '[redacted]' : (input.value || undefined),
|
||||
options: el.tagName === 'SELECT'
|
||||
? [...(el as HTMLSelectElement).options].map(o => ({ value: o.value, text: o.text }))
|
||||
: undefined,
|
||||
@@ -142,6 +179,7 @@ export async function handleReadCommand(
|
||||
case 'js': {
|
||||
const expr = args[0];
|
||||
if (!expr) throw new Error('Usage: browse js <expression>');
|
||||
if (bm) assertJsOriginAllowed(bm, page.url());
|
||||
const wrapped = wrapForEvaluate(expr);
|
||||
const result = await target.evaluate(wrapped);
|
||||
return typeof result === 'object' ? JSON.stringify(result, null, 2) : String(result ?? '');
|
||||
@@ -150,6 +188,7 @@ export async function handleReadCommand(
|
||||
case 'eval': {
|
||||
const filePath = args[0];
|
||||
if (!filePath) throw new Error('Usage: browse eval <js-file>');
|
||||
if (bm) assertJsOriginAllowed(bm, page.url());
|
||||
validateReadPath(filePath);
|
||||
if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
|
||||
const code = fs.readFileSync(filePath, 'utf-8');
|
||||
|
||||
+30
-3
@@ -35,6 +35,7 @@ import {
|
||||
import { validateTempPath } from './path-security';
|
||||
import { resolveConfig, ensureStateDir, readVersionHash } from './config';
|
||||
import { emitActivity, subscribe, getActivityAfter, getActivityHistory, getSubscriberCount } from './activity';
|
||||
import { initAuditLog, writeAuditEntry } from './audit';
|
||||
import { inspectElement, modifyStyle, resetModifications, getModificationHistory, detachSession, type InspectorResult } from './cdp-inspector';
|
||||
// Bun.spawn used instead of child_process.spawn (compiled bun binaries
|
||||
// fail posix_spawn on all executables including /bin/bash)
|
||||
@@ -47,6 +48,7 @@ import * as crypto from 'crypto';
|
||||
// ─── Config ─────────────────────────────────────────────────────
|
||||
const config = resolveConfig();
|
||||
ensureStateDir(config);
|
||||
initAuditLog(config.auditLog);
|
||||
|
||||
// ─── Auth ───────────────────────────────────────────────────────
|
||||
const AUTH_TOKEN = crypto.randomUUID();
|
||||
@@ -1013,7 +1015,7 @@ async function handleCommandInternal(
|
||||
await cleanupHiddenMarkers(page);
|
||||
}
|
||||
} else {
|
||||
result = await handleReadCommand(command, args, session);
|
||||
result = await handleReadCommand(command, args, session, browserManager);
|
||||
}
|
||||
} else if (WRITE_COMMANDS.has(command)) {
|
||||
result = await handleWriteCommand(command, args, session, browserManager);
|
||||
@@ -1088,13 +1090,14 @@ async function handleCommandInternal(
|
||||
}
|
||||
|
||||
// Activity: emit command_end (skipped for chain subcommands)
|
||||
const successDuration = Date.now() - startTime;
|
||||
if (!opts?.skipActivity) {
|
||||
emitActivity({
|
||||
type: 'command_end',
|
||||
command,
|
||||
args,
|
||||
url: browserManager.getCurrentUrl(),
|
||||
duration: Date.now() - startTime,
|
||||
duration: successDuration,
|
||||
status: 'ok',
|
||||
result: result,
|
||||
tabs: browserManager.getTabCount(),
|
||||
@@ -1103,6 +1106,17 @@ async function handleCommandInternal(
|
||||
});
|
||||
}
|
||||
|
||||
writeAuditEntry({
|
||||
ts: new Date().toISOString(),
|
||||
cmd: command,
|
||||
args: args.join(' '),
|
||||
origin: browserManager.getCurrentUrl(),
|
||||
durationMs: successDuration,
|
||||
status: 'ok',
|
||||
hasCookies: browserManager.hasCookieImports(),
|
||||
mode: browserManager.getConnectionMode(),
|
||||
});
|
||||
|
||||
browserManager.resetFailures();
|
||||
// Restore original active tab if we pinned to a specific one
|
||||
if (savedTabId !== null) {
|
||||
@@ -1120,13 +1134,14 @@ async function handleCommandInternal(
|
||||
}
|
||||
|
||||
// Activity: emit command_end (error) — skipped for chain subcommands
|
||||
const errorDuration = Date.now() - startTime;
|
||||
if (!opts?.skipActivity) {
|
||||
emitActivity({
|
||||
type: 'command_end',
|
||||
command,
|
||||
args,
|
||||
url: browserManager.getCurrentUrl(),
|
||||
duration: Date.now() - startTime,
|
||||
duration: errorDuration,
|
||||
status: 'error',
|
||||
error: err.message,
|
||||
tabs: browserManager.getTabCount(),
|
||||
@@ -1135,6 +1150,18 @@ async function handleCommandInternal(
|
||||
});
|
||||
}
|
||||
|
||||
writeAuditEntry({
|
||||
ts: new Date().toISOString(),
|
||||
cmd: command,
|
||||
args: args.join(' '),
|
||||
origin: browserManager.getCurrentUrl(),
|
||||
durationMs: errorDuration,
|
||||
status: 'error',
|
||||
error: err.message,
|
||||
hasCookies: browserManager.hasCookieImports(),
|
||||
mode: browserManager.getConnectionMode(),
|
||||
});
|
||||
|
||||
browserManager.incrementFailures();
|
||||
let errorMsg = wrapError(err);
|
||||
const hint = browserManager.getFailureHint();
|
||||
|
||||
@@ -7,6 +7,8 @@ export const BLOCKED_METADATA_HOSTS = new Set([
|
||||
'169.254.169.254', // AWS/GCP/Azure instance metadata
|
||||
'fe80::1', // IPv6 link-local — common metadata endpoint alias
|
||||
'::ffff:169.254.169.254', // IPv4-mapped IPv6 form of the metadata IP
|
||||
'::ffff:a9fe:a9fe', // Hex-encoded IPv4-mapped form (URL constructor normalizes to this)
|
||||
'::a9fe:a9fe', // Deprecated IPv4-compatible hex form
|
||||
'metadata.google.internal', // GCP metadata
|
||||
'metadata.azure.internal', // Azure IMDS
|
||||
]);
|
||||
|
||||
@@ -13,7 +13,8 @@ import { validateNavigationUrl } from './url-validation';
|
||||
import { validateOutputPath } from './path-security';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { TEMP_DIR } from './platform';
|
||||
import { TEMP_DIR, isPathWithin } from './platform';
|
||||
import { SAFE_DIRECTORIES } from './path-security';
|
||||
import { modifyStyle, undoModification, resetModifications, getModificationHistory } from './cdp-inspector';
|
||||
|
||||
/**
|
||||
@@ -441,16 +442,17 @@ export async function handleWriteCommand(
|
||||
case 'cookie-import': {
|
||||
const filePath = args[0];
|
||||
if (!filePath) throw new Error('Usage: browse cookie-import <json-file>');
|
||||
// Path validation — prevent reading arbitrary files
|
||||
if (path.isAbsolute(filePath)) {
|
||||
const safeDirs = [TEMP_DIR, process.cwd()];
|
||||
const resolved = path.resolve(filePath);
|
||||
if (!safeDirs.some(dir => isPathWithin(resolved, dir))) {
|
||||
throw new Error(`Path must be within: ${safeDirs.join(', ')}`);
|
||||
}
|
||||
// Path validation — resolve to absolute and check against safe dirs.
|
||||
// Fixes #707: relative paths previously bypassed the safe directory check.
|
||||
// Mirrors validateOutputPath() — resolves symlinks (e.g., macOS /tmp → /private/tmp).
|
||||
const resolved = path.resolve(filePath);
|
||||
let resolvedReal = resolved;
|
||||
try { resolvedReal = fs.realpathSync(resolved); } catch {
|
||||
// File may not exist yet — resolve parent dir instead
|
||||
try { resolvedReal = path.join(fs.realpathSync(path.dirname(resolved)), path.basename(resolved)); } catch {}
|
||||
}
|
||||
if (path.normalize(filePath).includes('..')) {
|
||||
throw new Error('Path traversal sequences (..) are not allowed');
|
||||
if (!SAFE_DIRECTORIES.some(dir => isPathWithin(resolvedReal, dir))) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
|
||||
const raw = fs.readFileSync(filePath, 'utf-8');
|
||||
@@ -476,20 +478,24 @@ export async function handleWriteCommand(
|
||||
}
|
||||
|
||||
await page.context().addCookies(cookies);
|
||||
const importedDomains = [...new Set(cookies.map((c: any) => c.domain).filter(Boolean))];
|
||||
if (importedDomains.length > 0) bm.trackCookieImportDomains(importedDomains);
|
||||
return `Loaded ${cookies.length} cookies from ${filePath}`;
|
||||
}
|
||||
|
||||
case 'cookie-import-browser': {
|
||||
// Two modes:
|
||||
// 1. Direct CLI import: cookie-import-browser <browser> --domain <domain> [--profile <profile>]
|
||||
// 2. Open picker UI: cookie-import-browser [browser]
|
||||
// Requires --domain (or --all to explicitly import everything).
|
||||
// 2. Open picker UI: cookie-import-browser [browser] (interactive domain selection)
|
||||
const browserArg = args[0];
|
||||
const domainIdx = args.indexOf('--domain');
|
||||
const profileIdx = args.indexOf('--profile');
|
||||
const hasAll = args.includes('--all');
|
||||
const profile = (profileIdx !== -1 && profileIdx + 1 < args.length) ? args[profileIdx + 1] : 'Default';
|
||||
|
||||
if (domainIdx !== -1 && domainIdx + 1 < args.length) {
|
||||
// Direct import mode — no UI
|
||||
// Direct import mode — scoped to specific domain
|
||||
const domain = args[domainIdx + 1];
|
||||
// Validate --domain against current page hostname to prevent cross-site cookie injection
|
||||
const pageHostname = new URL(page.url()).hostname;
|
||||
@@ -501,13 +507,35 @@ export async function handleWriteCommand(
|
||||
const result = await importCookies(browser, [domain], profile);
|
||||
if (result.cookies.length > 0) {
|
||||
await page.context().addCookies(result.cookies);
|
||||
bm.trackCookieImportDomains([domain]);
|
||||
}
|
||||
const msg = [`Imported ${result.count} cookies for ${domain} from ${browser}`];
|
||||
if (result.failed > 0) msg.push(`(${result.failed} failed to decrypt)`);
|
||||
return msg.join(' ');
|
||||
}
|
||||
|
||||
// Picker UI mode — open in user's browser
|
||||
if (hasAll) {
|
||||
// Explicit all-cookies import — requires --all flag as a deliberate opt-in.
|
||||
// Imports every non-expired cookie domain from the browser.
|
||||
const browser = browserArg || 'comet';
|
||||
const { listDomains } = await import('./cookie-import-browser');
|
||||
const { domains } = listDomains(browser, profile);
|
||||
const allDomainNames = domains.map((d: any) => d.domain);
|
||||
if (allDomainNames.length === 0) {
|
||||
return `No cookies found in ${browser} (profile: ${profile})`;
|
||||
}
|
||||
const result = await importCookies(browser, allDomainNames, profile);
|
||||
if (result.cookies.length > 0) {
|
||||
await page.context().addCookies(result.cookies);
|
||||
bm.trackCookieImportDomains(allDomainNames);
|
||||
}
|
||||
const msg = [`Imported ${result.count} cookies across ${Object.keys(result.domainCounts).length} domains from ${browser}`];
|
||||
msg.push('(used --all: all browser cookies imported, consider --domain for tighter scoping)');
|
||||
if (result.failed > 0) msg.push(`(${result.failed} failed to decrypt)`);
|
||||
return msg.join(' ');
|
||||
}
|
||||
|
||||
// Picker UI mode — open in user's browser for interactive domain selection
|
||||
const port = bm.serverPort;
|
||||
if (!port) throw new Error('Server port not available');
|
||||
|
||||
@@ -525,7 +553,7 @@ export async function handleWriteCommand(
|
||||
if (err?.code !== 'ENOENT' && !err?.message?.includes('spawn')) throw err;
|
||||
}
|
||||
|
||||
return `Cookie picker opened at http://127.0.0.1:${port}/cookie-picker\nDetected browsers: ${browsers.map(b => b.name).join(', ')}\nSelect domains to import, then close the picker when done.`;
|
||||
return `Cookie picker opened at http://127.0.0.1:${port}/cookie-picker\nDetected browsers: ${browsers.map(b => b.name).join(', ')}\nSelect domains to import, then close the picker when done.\n\nTip: For scripted imports, use --domain <domain> to scope cookies to a single domain.`;
|
||||
}
|
||||
|
||||
case 'style': {
|
||||
|
||||
@@ -1811,7 +1811,8 @@ describe('Path traversal prevention', () => {
|
||||
await handleWriteCommand('cookie-import', ['../../etc/shadow'], bm);
|
||||
expect(true).toBe(false);
|
||||
} catch (err: any) {
|
||||
expect(err.message).toContain('Path traversal');
|
||||
// Traversal blocked by safe-directory check (#707) or explicit .. check
|
||||
expect(err.message).toMatch(/Path must be within|Path traversal/);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -49,7 +49,7 @@ export function createSession(
|
||||
updatedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
fs.writeFileSync(sessionPath(id), JSON.stringify(session, null, 2));
|
||||
fs.writeFileSync(sessionPath(id), JSON.stringify(session, null, 2), { mode: 0o600 });
|
||||
return session;
|
||||
}
|
||||
|
||||
|
||||
@@ -208,7 +208,7 @@ if [ "$NEEDS_BUILD" -eq 1 ]; then
|
||||
log "Building browse binary..."
|
||||
(
|
||||
cd "$SOURCE_GSTACK_DIR"
|
||||
bun install
|
||||
bun install --frozen-lockfile 2>/dev/null || bun install
|
||||
bun run build
|
||||
)
|
||||
# Safety net: write .version if build script didn't (e.g., git not available during build)
|
||||
|
||||
Reference in New Issue
Block a user