mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 11:45:20 +02:00
fix(security): IPv6 ULA blocking, cookie redaction, per-tab cancel, targeted token (#664)
Community PR #664 by @mr-k-man (security audit round 1, new parts only). - IPv6 ULA prefix blocking (fc00::/7) in url-validation.ts with false-positive guard for hostnames like fd.example.com - Cookie value redaction for tokens, API keys, JWTs in browse cookies command - Per-tab cancel files in killAgent() replacing broken global kill-signal - design/serve.ts: realpathSync upgrade prevents symlink bypass in /api/reload - extension: targeted getToken handler replaces token-in-health-broadcast - Supabase migration 003: column-level GRANT restricts anon UPDATE scope - Telemetry sync: upsert error logging - 10 new tests for IPv6, cookie redaction, DNS rebinding, path traversal Co-Authored-By: mr-k-man <mr-k-man@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -122,6 +122,11 @@ case "$HTTP_CODE" in
|
||||
# Advance by SENT count (not inserted count) because we can't map inserted back to
|
||||
# source lines. If inserted==0, something is systemically wrong — don't advance.
|
||||
INSERTED="$(grep -o '"inserted":[0-9]*' "$RESP_FILE" 2>/dev/null | grep -o '[0-9]*' || echo "0")"
|
||||
# Check for upsert errors (installation tracking failures) — log but don't block cursor advance
|
||||
UPSERT_ERRORS="$(grep -o '"upsertErrors"' "$RESP_FILE" 2>/dev/null || true)"
|
||||
if [ -n "$UPSERT_ERRORS" ]; then
|
||||
echo "[gstack-telemetry-sync] Warning: installation upsert errors in response" >&2
|
||||
fi
|
||||
if [ "${INSERTED:-0}" -gt 0 ] 2>/dev/null; then
|
||||
NEW_CURSOR=$(( CURSOR + COUNT ))
|
||||
echo "$NEW_CURSOR" > "$CURSOR_FILE" 2>/dev/null || true
|
||||
|
||||
@@ -13,6 +13,10 @@ import * as path from 'path';
|
||||
import { TEMP_DIR, isPathWithin } from './platform';
|
||||
import { inspectElement, formatInspectorResult, getModificationHistory } from './cdp-inspector';
|
||||
|
||||
// Redaction patterns for sensitive cookie/storage values — exported for test coverage
|
||||
export const SENSITIVE_COOKIE_NAME = /(^|[_.-])(token|secret|key|password|credential|auth|jwt|session|csrf|sid)($|[_.-])|api.?key/i;
|
||||
export const SENSITIVE_COOKIE_VALUE = /^(eyJ|sk-|sk_live_|sk_test_|pk_live_|pk_test_|rk_live_|sk-ant-|ghp_|gho_|github_pat_|xox[bpsa]-|AKIA[A-Z0-9]{16}|AIza|SG\.|Bearer\s|sbp_)/;
|
||||
|
||||
/** Detect await keyword, ignoring comments. Accepted risk: await in string literals triggers wrapping (harmless). */
|
||||
function hasAwait(code: string): boolean {
|
||||
const stripped = code.replace(/\/\/.*$/gm, '').replace(/\/\*[\s\S]*?\*\//g, '');
|
||||
@@ -300,7 +304,14 @@ export async function handleReadCommand(
|
||||
|
||||
case 'cookies': {
|
||||
const cookies = await page.context().cookies();
|
||||
return JSON.stringify(cookies, null, 2);
|
||||
// Redact cookie values that look like secrets (consistent with storage redaction)
|
||||
const redacted = cookies.map(c => {
|
||||
if (SENSITIVE_COOKIE_NAME.test(c.name) || SENSITIVE_COOKIE_VALUE.test(c.value)) {
|
||||
return { ...c, value: `[REDACTED — ${c.value.length} chars]` };
|
||||
}
|
||||
return c;
|
||||
});
|
||||
return JSON.stringify(redacted, null, 2);
|
||||
}
|
||||
|
||||
case 'storage': {
|
||||
|
||||
+13
-10
@@ -572,7 +572,7 @@ function spawnClaude(userMessage: string, extensionUrl?: string | null, forTabId
|
||||
// Agent status transitions happen when we receive agent_done/agent_error events.
|
||||
}
|
||||
|
||||
function killAgent(): void {
|
||||
function killAgent(targetTabId?: number | null): void {
|
||||
if (agentProcess) {
|
||||
try { agentProcess.kill('SIGTERM'); } catch (err: any) {
|
||||
console.warn('[browse] Failed to SIGTERM agent:', err.message);
|
||||
@@ -581,17 +581,18 @@ function killAgent(): void {
|
||||
console.warn('[browse] Failed to SIGKILL agent:', err.message);
|
||||
} }, 3000);
|
||||
}
|
||||
// Signal the sidebar-agent worker to cancel via a per-tab cancel file.
|
||||
// Using per-tab files prevents race conditions where one agent's cancel
|
||||
// signal is consumed by a different tab's agent in concurrent mode.
|
||||
// When targetTabId is provided, only that tab's agent is cancelled.
|
||||
const cancelDir = path.join(process.env.HOME || '/tmp', '.gstack');
|
||||
const tabId = targetTabId ?? agentTabId ?? 0;
|
||||
const cancelFile = path.join(cancelDir, `sidebar-agent-cancel-${tabId}`);
|
||||
try { fs.writeFileSync(cancelFile, Date.now().toString()); } catch {}
|
||||
agentProcess = null;
|
||||
agentStartTime = null;
|
||||
currentMessage = null;
|
||||
agentStatus = 'idle';
|
||||
|
||||
// Signal sidebar-agent.ts to kill its active claude subprocess.
|
||||
// sidebar-agent runs in a separate non-compiled Bun process (posix_spawn
|
||||
// limitation). It polls the kill-signal file and terminates on any write.
|
||||
const agentQueue = process.env.SIDEBAR_QUEUE_PATH || path.join(process.env.HOME || '/tmp', '.gstack', 'sidebar-agent-queue.jsonl');
|
||||
const killFile = path.join(path.dirname(agentQueue), 'sidebar-agent-kill');
|
||||
try { fs.writeFileSync(killFile, String(Date.now())); } catch {}
|
||||
}
|
||||
|
||||
// Agent health check — detect hung processes
|
||||
@@ -1371,7 +1372,8 @@ async function start() {
|
||||
if (!validateAuth(req)) {
|
||||
return new Response(JSON.stringify({ error: 'Unauthorized' }), { status: 401, headers: { 'Content-Type': 'application/json' } });
|
||||
}
|
||||
killAgent();
|
||||
const killBody = await req.json().catch(() => ({}));
|
||||
killAgent(killBody.tabId ?? null);
|
||||
addChatEntry({ ts: new Date().toISOString(), role: 'agent', type: 'agent_error', error: 'Killed by user' });
|
||||
// Process next in queue
|
||||
if (messageQueue.length > 0) {
|
||||
@@ -1386,7 +1388,8 @@ async function start() {
|
||||
if (!validateAuth(req)) {
|
||||
return new Response(JSON.stringify({ error: 'Unauthorized' }), { status: 401, headers: { 'Content-Type': 'application/json' } });
|
||||
}
|
||||
killAgent();
|
||||
const stopBody = await req.json().catch(() => ({}));
|
||||
killAgent(stopBody.tabId ?? null);
|
||||
addChatEntry({ ts: new Date().toISOString(), role: 'agent', type: 'agent_error', error: 'Stopped by user' });
|
||||
return new Response(JSON.stringify({ ok: true, queuedMessages: messageQueue.length }), {
|
||||
status: 200, headers: { 'Content-Type': 'application/json' },
|
||||
|
||||
@@ -20,12 +20,18 @@ const SERVER_URL = `http://127.0.0.1:${SERVER_PORT}`;
|
||||
const POLL_MS = 200; // 200ms poll — keeps time-to-first-token low
|
||||
const B = process.env.BROWSE_BIN || path.resolve(__dirname, '../../.claude/skills/gstack/browse/dist/browse');
|
||||
|
||||
const CANCEL_DIR = path.join(process.env.HOME || '/tmp', '.gstack');
|
||||
function cancelFileForTab(tabId: number): string {
|
||||
return path.join(CANCEL_DIR, `sidebar-agent-cancel-${tabId}`);
|
||||
}
|
||||
|
||||
let lastLine = 0;
|
||||
let authToken: string | null = null;
|
||||
// Per-tab processing — each tab can run its own agent concurrently
|
||||
const processingTabs = new Set<number>();
|
||||
// Active claude subprocesses — keyed by tabId for targeted kill
|
||||
const activeProcs = new Map<number, ReturnType<typeof spawn>>();
|
||||
let activeProc: ReturnType<typeof spawn> | null = null;
|
||||
// Kill-file timestamp last seen — avoids double-kill on same write
|
||||
let lastKillTs = 0;
|
||||
|
||||
@@ -250,6 +256,10 @@ async function askClaude(queueEntry: any): Promise<void> {
|
||||
effectiveCwd = process.cwd();
|
||||
}
|
||||
|
||||
// Clear any stale cancel signal for this tab before starting
|
||||
const cancelFile = cancelFileForTab(tid);
|
||||
try { fs.unlinkSync(cancelFile); } catch {}
|
||||
|
||||
const proc = spawn('claude', claudeArgs, {
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
cwd: effectiveCwd,
|
||||
@@ -270,9 +280,23 @@ async function askClaude(queueEntry: any): Promise<void> {
|
||||
|
||||
// Track active procs so kill-file polling can terminate them
|
||||
activeProcs.set(tid, proc);
|
||||
activeProc = proc;
|
||||
|
||||
proc.stdin.end();
|
||||
|
||||
// Poll for per-tab cancel signal from server's killAgent()
|
||||
const cancelCheck = setInterval(() => {
|
||||
try {
|
||||
if (fs.existsSync(cancelFile)) {
|
||||
console.log(`[sidebar-agent] Cancel signal received for tab ${tid} — killing claude subprocess`);
|
||||
try { proc.kill('SIGTERM'); } catch {}
|
||||
setTimeout(() => { try { proc.kill('SIGKILL'); } catch {} }, 3000);
|
||||
fs.unlinkSync(cancelFile);
|
||||
clearInterval(cancelCheck);
|
||||
}
|
||||
} catch {}
|
||||
}, 500);
|
||||
|
||||
let buffer = '';
|
||||
|
||||
proc.stdout.on('data', (data: Buffer) => {
|
||||
@@ -293,6 +317,8 @@ async function askClaude(queueEntry: any): Promise<void> {
|
||||
});
|
||||
|
||||
proc.on('close', (code) => {
|
||||
clearInterval(cancelCheck);
|
||||
activeProc = null;
|
||||
activeProcs.delete(tid);
|
||||
if (buffer.trim()) {
|
||||
try { handleStreamEvent(JSON.parse(buffer), tid); } catch (err: any) {
|
||||
@@ -310,6 +336,8 @@ async function askClaude(queueEntry: any): Promise<void> {
|
||||
});
|
||||
|
||||
proc.on('error', (err) => {
|
||||
clearInterval(cancelCheck);
|
||||
activeProc = null;
|
||||
const errorMsg = stderrBuffer.trim()
|
||||
? `${err.message}\nstderr: ${stderrBuffer.trim().slice(-500)}`
|
||||
: err.message;
|
||||
|
||||
@@ -3,15 +3,34 @@
|
||||
* Localhost and private IPs are allowed (primary use case: QA testing local dev servers).
|
||||
*/
|
||||
|
||||
const BLOCKED_METADATA_HOSTS = new Set([
|
||||
'169.254.169.254', // AWS/GCP/Azure instance metadata (IPv4 link-local)
|
||||
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
|
||||
'fd00::', // IPv6 unique local (metadata in some cloud setups)
|
||||
'::ffff:169.254.169.254', // IPv4-mapped IPv6 form of the metadata IP
|
||||
'metadata.google.internal', // GCP metadata
|
||||
'metadata.azure.internal', // Azure IMDS
|
||||
]);
|
||||
|
||||
/**
|
||||
* IPv6 prefixes to block (CIDR-style). Any address starting with these
|
||||
* hex prefixes is rejected. Covers the full ULA range (fc00::/7 = fc00:: and fd00::).
|
||||
*/
|
||||
const BLOCKED_IPV6_PREFIXES = ['fc', 'fd'];
|
||||
|
||||
/**
|
||||
* Check if an IPv6 address falls within a blocked prefix range.
|
||||
* Handles the full ULA range (fc00::/7), not just the exact literal fd00::.
|
||||
* Only matches actual IPv6 addresses (must contain ':'), not hostnames
|
||||
* like fd.example.com or fcustomer.com.
|
||||
*/
|
||||
function isBlockedIpv6(addr: string): boolean {
|
||||
const normalized = addr.toLowerCase().replace(/^\[|\]$/g, '');
|
||||
// Must contain a colon to be an IPv6 address — avoids false positives on
|
||||
// hostnames like fd.example.com or fcustomer.com
|
||||
if (!normalized.includes(':')) return false;
|
||||
return BLOCKED_IPV6_PREFIXES.some(prefix => normalized.startsWith(prefix));
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize hostname for blocklist comparison:
|
||||
* - Strip trailing dot (DNS fully-qualified notation)
|
||||
@@ -37,7 +56,7 @@ function isMetadataIp(hostname: string): boolean {
|
||||
try {
|
||||
const probe = new URL(`http://${hostname}`);
|
||||
const normalized = probe.hostname;
|
||||
if (BLOCKED_METADATA_HOSTS.has(normalized)) return true;
|
||||
if (BLOCKED_METADATA_HOSTS.has(normalized) || isBlockedIpv6(normalized)) return true;
|
||||
// Also check after stripping trailing dot
|
||||
if (normalized.endsWith('.') && BLOCKED_METADATA_HOSTS.has(normalized.slice(0, -1))) return true;
|
||||
} catch {
|
||||
@@ -69,7 +88,7 @@ async function resolvesToBlockedIp(hostname: string): Promise<boolean> {
|
||||
const v6Check = resolve6(hostname).then(
|
||||
(addresses) => addresses.some(addr => {
|
||||
const normalized = addr.toLowerCase();
|
||||
return BLOCKED_METADATA_HOSTS.has(normalized) ||
|
||||
return BLOCKED_METADATA_HOSTS.has(normalized) || isBlockedIpv6(normalized) ||
|
||||
// fe80::/10 is link-local — always block (covers all fe80:: addresses)
|
||||
normalized.startsWith('fe80:');
|
||||
}),
|
||||
@@ -100,7 +119,7 @@ export async function validateNavigationUrl(url: string): Promise<void> {
|
||||
|
||||
const hostname = normalizeHostname(parsed.hostname.toLowerCase());
|
||||
|
||||
if (BLOCKED_METADATA_HOSTS.has(hostname) || isMetadataIp(hostname)) {
|
||||
if (BLOCKED_METADATA_HOSTS.has(hostname) || isMetadataIp(hostname) || isBlockedIpv6(hostname)) {
|
||||
throw new Error(
|
||||
`Blocked: ${parsed.hostname} is a cloud metadata endpoint. Access is denied for security.`
|
||||
);
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
import { validateOutputPath } from '../src/meta-commands';
|
||||
import { validateReadPath } from '../src/read-commands';
|
||||
import { readFileSync, symlinkSync, unlinkSync, writeFileSync } from 'fs';
|
||||
import { validateReadPath, SENSITIVE_COOKIE_NAME, SENSITIVE_COOKIE_VALUE } from '../src/read-commands';
|
||||
import { BLOCKED_METADATA_HOSTS } from '../src/url-validation';
|
||||
import { readFileSync, symlinkSync, unlinkSync, writeFileSync, realpathSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
@@ -109,3 +110,85 @@ describe('validateReadPath', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateOutputPath — symlink resolution', () => {
|
||||
it('blocks symlink inside /tmp pointing outside safe dirs', () => {
|
||||
const linkPath = join(tmpdir(), 'test-output-symlink-' + Date.now() + '.png');
|
||||
try {
|
||||
symlinkSync('/etc/crontab', linkPath);
|
||||
expect(() => validateOutputPath(linkPath)).toThrow(/Path must be within/);
|
||||
} finally {
|
||||
try { unlinkSync(linkPath); } catch {}
|
||||
}
|
||||
});
|
||||
|
||||
it('allows symlink inside /tmp pointing to another /tmp path', () => {
|
||||
// Use /tmp (TEMP_DIR on macOS/Linux), not os.tmpdir() which may be a different path
|
||||
const realTmp = realpathSync('/tmp');
|
||||
const targetPath = join(realTmp, 'test-output-real-' + Date.now() + '.png');
|
||||
const linkPath = join(realTmp, 'test-output-link-' + Date.now() + '.png');
|
||||
try {
|
||||
writeFileSync(targetPath, '');
|
||||
symlinkSync(targetPath, linkPath);
|
||||
expect(() => validateOutputPath(linkPath)).not.toThrow();
|
||||
} finally {
|
||||
try { unlinkSync(linkPath); } catch {}
|
||||
try { unlinkSync(targetPath); } catch {}
|
||||
}
|
||||
});
|
||||
|
||||
it('blocks new file in symlinked directory pointing outside', () => {
|
||||
const linkDir = join(tmpdir(), 'test-dirlink-' + Date.now());
|
||||
try {
|
||||
symlinkSync('/etc', linkDir);
|
||||
expect(() => validateOutputPath(join(linkDir, 'evil.png'))).toThrow(/Path must be within/);
|
||||
} finally {
|
||||
try { unlinkSync(linkDir); } catch {}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('cookie redaction — production patterns', () => {
|
||||
it('detects sensitive cookie names', () => {
|
||||
expect(SENSITIVE_COOKIE_NAME.test('session_id')).toBe(true);
|
||||
expect(SENSITIVE_COOKIE_NAME.test('auth_token')).toBe(true);
|
||||
expect(SENSITIVE_COOKIE_NAME.test('csrf-token')).toBe(true);
|
||||
expect(SENSITIVE_COOKIE_NAME.test('api_key')).toBe(true);
|
||||
expect(SENSITIVE_COOKIE_NAME.test('jwt.payload')).toBe(true);
|
||||
});
|
||||
|
||||
it('ignores non-sensitive cookie names', () => {
|
||||
expect(SENSITIVE_COOKIE_NAME.test('theme')).toBe(false);
|
||||
expect(SENSITIVE_COOKIE_NAME.test('locale')).toBe(false);
|
||||
expect(SENSITIVE_COOKIE_NAME.test('_ga')).toBe(false);
|
||||
});
|
||||
|
||||
it('detects sensitive cookie value prefixes', () => {
|
||||
expect(SENSITIVE_COOKIE_VALUE.test('eyJhbGciOiJIUzI1NiJ9')).toBe(true); // JWT
|
||||
expect(SENSITIVE_COOKIE_VALUE.test('sk-ant-abc123')).toBe(true); // Anthropic
|
||||
expect(SENSITIVE_COOKIE_VALUE.test('ghp_xxxxxxxxxxxx')).toBe(true); // GitHub PAT
|
||||
expect(SENSITIVE_COOKIE_VALUE.test('xoxb-token')).toBe(true); // Slack
|
||||
});
|
||||
|
||||
it('ignores non-sensitive values', () => {
|
||||
expect(SENSITIVE_COOKIE_VALUE.test('dark')).toBe(false);
|
||||
expect(SENSITIVE_COOKIE_VALUE.test('en-US')).toBe(false);
|
||||
expect(SENSITIVE_COOKIE_VALUE.test('1234567890')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DNS rebinding — production blocklist', () => {
|
||||
it('blocks fd00:: IPv6 metadata address via validateNavigationUrl', async () => {
|
||||
const { validateNavigationUrl } = await import('../src/url-validation');
|
||||
await expect(validateNavigationUrl('http://[fd00::]/')).rejects.toThrow(/cloud metadata/i);
|
||||
});
|
||||
|
||||
it('blocks AWS/GCP IPv4 metadata address', () => {
|
||||
expect(BLOCKED_METADATA_HOSTS.has('169.254.169.254')).toBe(true);
|
||||
});
|
||||
|
||||
it('does not block normal addresses', () => {
|
||||
expect(BLOCKED_METADATA_HOSTS.has('8.8.8.8')).toBe(false);
|
||||
expect(BLOCKED_METADATA_HOSTS.has('2001:4860:4860::8888')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -62,11 +62,53 @@ describe('validateNavigationUrl', () => {
|
||||
await expect(validateNavigationUrl('http://0251.0376.0251.0376/')).rejects.toThrow(/cloud metadata/i);
|
||||
});
|
||||
|
||||
it('blocks IPv6 metadata with brackets', async () => {
|
||||
it('blocks IPv6 metadata with brackets (fd00::)', async () => {
|
||||
await expect(validateNavigationUrl('http://[fd00::]/')).rejects.toThrow(/cloud metadata/i);
|
||||
});
|
||||
|
||||
it('blocks IPv6 ULA fd00::1 (not just fd00::)', async () => {
|
||||
await expect(validateNavigationUrl('http://[fd00::1]/')).rejects.toThrow(/cloud metadata/i);
|
||||
});
|
||||
|
||||
it('blocks IPv6 ULA fd12:3456::1', async () => {
|
||||
await expect(validateNavigationUrl('http://[fd12:3456::1]/')).rejects.toThrow(/cloud metadata/i);
|
||||
});
|
||||
|
||||
it('blocks IPv6 ULA fc00:: (full fc00::/7 range)', async () => {
|
||||
await expect(validateNavigationUrl('http://[fc00::]/')).rejects.toThrow(/cloud metadata/i);
|
||||
});
|
||||
|
||||
it('does not block hostnames starting with fd (e.g. fd.example.com)', async () => {
|
||||
await expect(validateNavigationUrl('https://fd.example.com/')).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not block hostnames starting with fc (e.g. fcustomer.com)', async () => {
|
||||
await expect(validateNavigationUrl('https://fcustomer.com/')).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('throws on malformed URLs', async () => {
|
||||
await expect(validateNavigationUrl('not-a-url')).rejects.toThrow(/Invalid URL/i);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateNavigationUrl — restoreState coverage', () => {
|
||||
it('blocks file:// URLs that could appear in saved state', async () => {
|
||||
await expect(validateNavigationUrl('file:///etc/passwd')).rejects.toThrow(/scheme.*not allowed/i);
|
||||
});
|
||||
|
||||
it('blocks chrome:// URLs that could appear in saved state', async () => {
|
||||
await expect(validateNavigationUrl('chrome://settings')).rejects.toThrow(/scheme.*not allowed/i);
|
||||
});
|
||||
|
||||
it('blocks metadata IPs that could be injected into state files', async () => {
|
||||
await expect(validateNavigationUrl('http://169.254.169.254/latest/meta-data/')).rejects.toThrow(/cloud metadata/i);
|
||||
});
|
||||
|
||||
it('allows normal https URLs from saved state', async () => {
|
||||
await expect(validateNavigationUrl('https://example.com/page')).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('allows localhost URLs from saved state', async () => {
|
||||
await expect(validateNavigationUrl('http://localhost:3000/app')).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
+11
-7
@@ -55,6 +55,10 @@ export async function serve(options: ServeOptions): Promise<void> {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// Security: anchor all file reads to the initial HTML's directory.
|
||||
// Prevents /api/reload from reading arbitrary files via path traversal.
|
||||
const allowedDir = fs.realpathSync(path.dirname(path.resolve(html)));
|
||||
|
||||
let htmlContent = fs.readFileSync(html, "utf-8");
|
||||
let state: ServerState = "serving";
|
||||
let timeoutTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
@@ -185,19 +189,19 @@ export async function serve(options: ServeOptions): Promise<void> {
|
||||
);
|
||||
}
|
||||
|
||||
// Validate path is within cwd or temp directory
|
||||
const resolved = path.resolve(newHtmlPath);
|
||||
const safeDirs = [process.cwd(), os.tmpdir()];
|
||||
const isSafe = safeDirs.some(dir => resolved.startsWith(dir + path.sep) || resolved === dir);
|
||||
if (!isSafe) {
|
||||
// Security: resolve symlinks and validate the reload path is within the
|
||||
// allowed directory (anchored to the initial HTML file's parent).
|
||||
// Prevents path traversal via /api/reload reading arbitrary files.
|
||||
const resolvedReload = fs.realpathSync(path.resolve(newHtmlPath));
|
||||
if (!resolvedReload.startsWith(allowedDir + path.sep) && resolvedReload !== allowedDir) {
|
||||
return Response.json(
|
||||
{ error: `Path must be within working directory or temp` },
|
||||
{ error: `Path must be within: ${allowedDir}` },
|
||||
{ status: 403 }
|
||||
);
|
||||
}
|
||||
|
||||
// Swap the HTML content
|
||||
htmlContent = fs.readFileSync(newHtmlPath, "utf-8");
|
||||
htmlContent = fs.readFileSync(resolvedReload, "utf-8");
|
||||
state = "serving";
|
||||
|
||||
console.error(`SERVE_RELOADED: html=${newHtmlPath}`);
|
||||
|
||||
@@ -274,6 +274,103 @@ describe('Serve HTTP endpoints', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Path traversal protection in /api/reload ─────────────────────
|
||||
|
||||
describe('Serve /api/reload — path traversal protection', () => {
|
||||
let server: ReturnType<typeof Bun.serve>;
|
||||
let baseUrl: string;
|
||||
let htmlContent: string;
|
||||
let allowedDir: string;
|
||||
|
||||
beforeAll(() => {
|
||||
// Production-equivalent allowedDir anchored to tmpDir
|
||||
allowedDir = fs.realpathSync(tmpDir);
|
||||
htmlContent = fs.readFileSync(boardHtml, 'utf-8');
|
||||
|
||||
// This server mirrors the production serve() with the path validation fix
|
||||
server = Bun.serve({
|
||||
port: 0,
|
||||
fetch(req) {
|
||||
const url = new URL(req.url);
|
||||
|
||||
if (req.method === 'GET' && url.pathname === '/') {
|
||||
return new Response(htmlContent, {
|
||||
headers: { 'Content-Type': 'text/html; charset=utf-8' },
|
||||
});
|
||||
}
|
||||
|
||||
if (req.method === 'POST' && url.pathname === '/api/reload') {
|
||||
return (async () => {
|
||||
let body: any;
|
||||
try { body = await req.json(); } catch { return Response.json({ error: 'Invalid JSON' }, { status: 400 }); }
|
||||
if (!body.html || !fs.existsSync(body.html)) {
|
||||
return Response.json({ error: `HTML file not found: ${body.html}` }, { status: 400 });
|
||||
}
|
||||
// Production path validation — same as design/src/serve.ts
|
||||
const resolvedReload = fs.realpathSync(path.resolve(body.html));
|
||||
if (!resolvedReload.startsWith(allowedDir + path.sep) && resolvedReload !== allowedDir) {
|
||||
return Response.json({ error: `Path must be within: ${allowedDir}` }, { status: 403 });
|
||||
}
|
||||
htmlContent = fs.readFileSync(resolvedReload, 'utf-8');
|
||||
return Response.json({ reloaded: true });
|
||||
})();
|
||||
}
|
||||
|
||||
return new Response('Not found', { status: 404 });
|
||||
},
|
||||
});
|
||||
baseUrl = `http://localhost:${server.port}`;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
server.stop();
|
||||
});
|
||||
|
||||
test('blocks reload with path outside allowed directory', async () => {
|
||||
const res = await fetch(`${baseUrl}/api/reload`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ html: '/etc/passwd' }),
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
const data = await res.json();
|
||||
expect(data.error).toContain('Path must be within');
|
||||
});
|
||||
|
||||
test('blocks reload with symlink pointing outside allowed directory', async () => {
|
||||
const linkPath = path.join(tmpDir, 'evil-link.html');
|
||||
try {
|
||||
fs.symlinkSync('/etc/passwd', linkPath);
|
||||
const res = await fetch(`${baseUrl}/api/reload`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ html: linkPath }),
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
} finally {
|
||||
try { fs.unlinkSync(linkPath); } catch {}
|
||||
}
|
||||
});
|
||||
|
||||
test('allows reload with file inside allowed directory', async () => {
|
||||
const goodPath = path.join(tmpDir, 'safe-board.html');
|
||||
fs.writeFileSync(goodPath, '<html><body>Safe reload</body></html>');
|
||||
|
||||
const res = await fetch(`${baseUrl}/api/reload`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ html: goodPath }),
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
const data = await res.json();
|
||||
expect(data.reloaded).toBe(true);
|
||||
|
||||
// Verify the new content is served
|
||||
const page = await fetch(baseUrl);
|
||||
expect(await page.text()).toContain('Safe reload');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Full lifecycle: regeneration round-trip ──────────────────────
|
||||
|
||||
describe('Full regeneration lifecycle', () => {
|
||||
|
||||
+15
-4
@@ -87,8 +87,8 @@ function setConnected(healthData) {
|
||||
chrome.action.setBadgeBackgroundColor({ color: '#F59E0B' });
|
||||
chrome.action.setBadgeText({ text: ' ' });
|
||||
|
||||
// Broadcast health to popup and side panel (include token for sidepanel auth)
|
||||
chrome.runtime.sendMessage({ type: 'health', data: { ...healthData, token: authToken } }).catch((err) => {
|
||||
// Broadcast health to popup and side panel (token excluded — use getToken message instead)
|
||||
chrome.runtime.sendMessage({ type: 'health', data: healthData }).catch((err) => {
|
||||
console.debug('[gstack bg] No listener for health broadcast:', err.message);
|
||||
});
|
||||
|
||||
@@ -285,7 +285,7 @@ chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
}
|
||||
|
||||
const ALLOWED_TYPES = new Set([
|
||||
'getPort', 'setPort', 'getServerUrl', 'fetchRefs',
|
||||
'getPort', 'setPort', 'getServerUrl', 'getToken', 'fetchRefs',
|
||||
'openSidePanel', 'sidebarOpened', 'command', 'sidebar-command',
|
||||
// Inspector message types
|
||||
'startInspector', 'stopInspector', 'elementPicked', 'pickerCancelled',
|
||||
@@ -315,7 +315,18 @@ chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
return true;
|
||||
}
|
||||
|
||||
// getToken handler removed — token distributed via health broadcast
|
||||
// Token delivered via targeted sendResponse, not broadcast — limits exposure.
|
||||
// Only respond to extension pages (sidepanel/popup) — content scripts have
|
||||
// sender.tab set, so reject those to prevent token access from injected contexts.
|
||||
if (msg.type === 'getToken') {
|
||||
if (sender.tab) {
|
||||
console.warn('[gstack] Rejected getToken from content script context');
|
||||
sendResponse({ token: null });
|
||||
} else {
|
||||
sendResponse({ token: authToken });
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
if (msg.type === 'fetchRefs') {
|
||||
fetchAndRelayRefs().then(() => sendResponse({ ok: true }));
|
||||
|
||||
@@ -1570,7 +1570,10 @@ chrome.runtime.onMessage.addListener((msg) => {
|
||||
if (msg.type === 'health') {
|
||||
if (msg.data) {
|
||||
const url = `http://127.0.0.1:${msg.data.port || 34567}`;
|
||||
updateConnection(url, msg.data.token);
|
||||
// Request token via targeted sendResponse (not broadcast) to limit exposure
|
||||
chrome.runtime.sendMessage({ type: 'getToken' }, (resp) => {
|
||||
updateConnection(url, resp?.token || null);
|
||||
});
|
||||
applyChatEnabled(!!msg.data.chatEnabled);
|
||||
} else {
|
||||
updateConnection(null);
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
-- 003_installations_upsert_policy.sql
|
||||
-- Re-add a scoped UPDATE policy for installations so the telemetry-ingest
|
||||
-- edge function can upsert (update last_seen) using the caller's anon key
|
||||
-- instead of the service role key.
|
||||
--
|
||||
-- Migration 002 dropped the overly broad "anon_update_last_seen" policy
|
||||
-- (which allowed UPDATE on ALL columns). This replacement uses:
|
||||
-- 1. An RLS policy to allow UPDATE (required for any row access)
|
||||
-- 2. Column-level GRANT to restrict anon to only the tracking columns
|
||||
-- the edge function actually writes (last_seen, gstack_version, os)
|
||||
--
|
||||
-- This means anon callers cannot UPDATE first_seen or installation_id,
|
||||
-- closing the residual risk from the broad RLS-only approach.
|
||||
|
||||
-- RLS policy: allow UPDATE on rows (required for PostgREST/upsert)
|
||||
CREATE POLICY "anon_update_tracking" ON installations
|
||||
FOR UPDATE
|
||||
USING (true)
|
||||
WITH CHECK (true);
|
||||
|
||||
-- Column-level restriction: anon can only UPDATE these three columns.
|
||||
-- PostgreSQL GRANT UPDATE (col, ...) is enforced at the query level —
|
||||
-- any UPDATE touching other columns will be rejected with a permission error.
|
||||
REVOKE UPDATE ON installations FROM anon;
|
||||
GRANT UPDATE (last_seen, gstack_version, os) ON installations TO anon;
|
||||
Reference in New Issue
Block a user