mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-09 10:18:32 +01:00
browse: redact query values from the audit origin and network log (GST-1635) (#4082)
* browse: redact query values from the audit origin and network log, scrub short digit pieces (GST-1635) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: build the userinfo test URL at runtime so the credential scan passes (GST-1635) The literal user:password URL tripped creds.basic_auth_url in the pregate and in CI's quality credential scan. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * browse: redact a query piece with an empty or '=' value whole; fragment names what stays recorded (GST-1635) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
115e245ace
commit
9ceec3f05e
5 files changed
+116
-15
No files matched your search
+34
-7
@@ -5,7 +5,8 @@
|
||||
* 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 (typed values redacted, then truncated), page origin
|
||||
* - timestamp, command, args (typed values redacted, then truncated), page
|
||||
* origin (scheme, host and path, with every query value redacted)
|
||||
* - duration, status (ok/error), error message if any
|
||||
* - whether cookies were imported (elevated security context)
|
||||
* - connection mode (headless/headed)
|
||||
@@ -14,7 +15,8 @@
|
||||
* value, typed text, a js expression, a prompt answer, a cookie, header,
|
||||
* storage or skill-arg value) is replaced with [REDACTED] before truncation,
|
||||
* and the error text is scrubbed of the same values. Navigation and click
|
||||
* args are kept as typed.
|
||||
* args are kept as typed. The page origin keeps its query parameter names but
|
||||
* not their values, since a GET form puts whatever the user typed there.
|
||||
*
|
||||
* All writes are best-effort — audit failures never cause command failures.
|
||||
*/
|
||||
@@ -138,9 +140,34 @@ export function redactCommandArgs(rawCmd: string, args: string[]): RedactedArgs
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove every redacted value, its JSON-escaped form and each 4+ char piece of it
|
||||
* (split on whitespace and on punctuation, since a js error echoes a bare
|
||||
* identifier) from text. Needles under 6 chars match only as whole tokens, so a
|
||||
* Keep a URL's scheme, host and path and its query parameter names. Userinfo
|
||||
* and the fragment are dropped and every query value becomes [REDACTED]: page
|
||||
* and request URLs carry GET form fields and tokens under names no list covers.
|
||||
*/
|
||||
export function redactUrl(raw: string): string {
|
||||
let url: URL;
|
||||
try {
|
||||
url = new URL(raw);
|
||||
} catch {
|
||||
return raw.split(/[?#]/)[0];
|
||||
}
|
||||
const query = url.search.slice(1).split('&').filter(Boolean).map(piece => {
|
||||
const eq = piece.indexOf('=');
|
||||
return eq > 0 && /[^=]/.test(piece.slice(eq + 1)) ? piece.slice(0, eq + 1) + REDACTED : REDACTED;
|
||||
});
|
||||
url.username = '';
|
||||
url.password = '';
|
||||
url.hash = '';
|
||||
url.search = '';
|
||||
return query.length ? `${url.href}?${query.join('&')}` : url.href;
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove every redacted value, its JSON-escaped form and each piece of it (split
|
||||
* on whitespace and on punctuation, since a js error echoes a bare identifier)
|
||||
* from text. A piece counts when it has 4+ chars, or 2-3 chars with a digit (a
|
||||
* CVV, an expiry month); shorter word pieces like "a" or "is" would shred
|
||||
* ordinary error prose. Needles under 6 chars match only as whole tokens, so a
|
||||
* short value neither shreds the error nor shows through the text around it.
|
||||
*/
|
||||
export function scrubSecrets(text: string, secrets: string[]): string {
|
||||
@@ -150,7 +177,7 @@ export function scrubSecrets(text: string, secrets: string[]): string {
|
||||
needles.add(secret);
|
||||
needles.add(JSON.stringify(secret).slice(1, -1));
|
||||
for (const piece of [...secret.split(/\s+/), ...secret.split(/[^A-Za-z0-9_$.-]+/)]) {
|
||||
if (piece.length >= 4) {
|
||||
if (piece.length >= 4 || (piece.length >= 2 && /\d/.test(piece))) {
|
||||
needles.add(piece);
|
||||
needles.add(JSON.stringify(piece).slice(1, -1));
|
||||
}
|
||||
@@ -187,7 +214,7 @@ export function writeAuditEntry(entry: AuditEntry): void {
|
||||
ts: entry.ts,
|
||||
cmd: entry.cmd,
|
||||
args: truncatedArgs,
|
||||
origin: entry.origin,
|
||||
origin: redactUrl(entry.origin),
|
||||
durationMs: entry.durationMs,
|
||||
status: entry.status,
|
||||
hasCookies: entry.hasCookies,
|
||||
|
||||
@@ -15,6 +15,8 @@
|
||||
* totalAdded keeps incrementing past capacity (flush cursor)
|
||||
*/
|
||||
|
||||
import { redactUrl } from './audit';
|
||||
|
||||
// ─── CircularBuffer ─────────────────────────────────────────
|
||||
|
||||
export class CircularBuffer<T> {
|
||||
@@ -136,6 +138,11 @@ export function addDialogEntry(entry: DialogEntry) {
|
||||
dialogBuffer.push(entry);
|
||||
}
|
||||
|
||||
/** One browse-network.log line. The URL goes through redactUrl, so its query values, userinfo and fragment never reach disk. */
|
||||
export function formatNetworkLogLine(e: NetworkEntry): string {
|
||||
return `[${new Date(e.timestamp).toISOString()}] ${e.method} ${redactUrl(e.url)} → ${e.status || 'pending'} (${e.duration || '?'}ms, ${e.size || '?'}B)`;
|
||||
}
|
||||
|
||||
/** One browse-dialog.log line. The prompt response is text the agent typed (dialog-accept), so it never reaches disk. */
|
||||
export function formatDialogLogLine(e: DialogEntry): string {
|
||||
const response = e.response ? ' "[REDACTED]"' : '';
|
||||
|
||||
@@ -608,7 +608,7 @@ function generateHelpText(): string {
|
||||
}
|
||||
|
||||
// ─── Buffer (from buffers.ts) ────────────────────────────────────
|
||||
import { consoleBuffer, networkBuffer, dialogBuffer, addConsoleEntry, addNetworkEntry, addDialogEntry, formatDialogLogLine, type LogEntry, type NetworkEntry, type DialogEntry } from './buffers';
|
||||
import { consoleBuffer, networkBuffer, dialogBuffer, addConsoleEntry, addNetworkEntry, addDialogEntry, formatDialogLogLine, formatNetworkLogLine, type LogEntry, type NetworkEntry, type DialogEntry } from './buffers';
|
||||
|
||||
export { consoleBuffer, networkBuffer, dialogBuffer, addConsoleEntry, addNetworkEntry, addDialogEntry, type LogEntry, type NetworkEntry, type DialogEntry };
|
||||
|
||||
@@ -664,10 +664,7 @@ async function flushBuffers() {
|
||||
const newNetworkCount = networkBuffer.totalAdded - lastNetworkFlushed;
|
||||
if (newNetworkCount > 0) {
|
||||
const entries = networkBuffer.last(Math.min(newNetworkCount, networkBuffer.length));
|
||||
const lines = entries.map(e =>
|
||||
`[${new Date(e.timestamp).toISOString()}] ${e.method} ${e.url} → ${e.status || 'pending'} (${e.duration || '?'}ms, ${e.size || '?'}B)`
|
||||
).join('\n') + '\n';
|
||||
appendSecureFile(NETWORK_LOG_PATH, lines);
|
||||
appendSecureFile(NETWORK_LOG_PATH, entries.map(formatNetworkLogLine).join('\n') + '\n');
|
||||
lastNetworkFlushed = networkBuffer.totalAdded;
|
||||
}
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
|
||||
import { initAuditLog, writeAuditEntry } from '../src/audit';
|
||||
import { formatDialogLogLine } from '../src/buffers';
|
||||
import { formatDialogLogLine, formatNetworkLogLine } from '../src/buffers';
|
||||
import { handleMetaCommand } from '../src/meta-commands';
|
||||
|
||||
// GST-1631: the audit log is plain JSONL in the state dir, so every value a
|
||||
@@ -25,12 +25,12 @@ afterEach(() => {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function write(cmd: string, args: string[], error?: string) {
|
||||
function write(cmd: string, args: string[], error?: string, origin = 'https://example.com/login') {
|
||||
writeAuditEntry({
|
||||
ts: '2026-11-06T00:00:00.000Z',
|
||||
cmd,
|
||||
args,
|
||||
origin: 'https://example.com/login',
|
||||
origin,
|
||||
durationMs: 1,
|
||||
status: error ? 'error' : 'ok',
|
||||
error,
|
||||
@@ -233,6 +233,75 @@ describe('error text is scrubbed of the typed values', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('short value pieces in error text (GST-1635)', () => {
|
||||
it('scrubs a 3-digit piece of a js expression, such as a CVV, as a whole token', () => {
|
||||
write('js', ["pay('4111111111111111', '123')"], 'Error: card 4111111111111111 cvv 123 rejected at eval:1:5');
|
||||
expect(line().error).toBe('Error: card [REDACTED] cvv [REDACTED] rejected at eval:1:5');
|
||||
});
|
||||
|
||||
it('scrubs a 2-digit piece but never one inside a longer number', () => {
|
||||
write('fill', ['#exp', '07 / 29'], 'invalid expiry 07/29, year 2029');
|
||||
expect(line().error).toBe('invalid expiry [REDACTED]/[REDACTED], year 2029');
|
||||
});
|
||||
|
||||
it('leaves short word pieces and single digits alone, so ordinary error text survives', () => {
|
||||
write('js', ['go(a, 1).is.of'], 'TypeError: a is not a function at eval:1:9; go to page 1');
|
||||
expect(line().error).toBe('TypeError: a is not a function at eval:1:9; go to page 1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('audit origin keeps scheme, host and path (GST-1635)', () => {
|
||||
it('redacts every query value a GET form put in the page URL', () => {
|
||||
write('click', ['@e3'], undefined, 'https://example.com/login?user=a&password=hunter2pw&cvv=123');
|
||||
expect(raw()).not.toMatch(/hunter2pw|cvv=123|user=a\b/);
|
||||
expect(line().origin).toBe('https://example.com/login?user=[REDACTED]&password=[REDACTED]&cvv=[REDACTED]');
|
||||
});
|
||||
|
||||
it('drops a fragment and userinfo, and redacts a bare query piece', () => {
|
||||
const userinfo = 'bob:s3cretpw';
|
||||
write('click', ['@e3'], undefined, `https://${userinfo}@example.com/cb?hunter2pw#access_token=tok_abcdef`);
|
||||
expect(raw()).not.toMatch(/s3cretpw|hunter2pw|tok_abcdef|bob/);
|
||||
expect(line().origin).toBe('https://example.com/cb?[REDACTED]');
|
||||
});
|
||||
|
||||
it('redacts a whole piece whose value is empty or only "=", so a padded bare token keeps nothing', () => {
|
||||
write('click', ['@e3'], undefined, 'https://example.com/cb?c2VjcmV0==&flag=&q=x');
|
||||
expect(raw()).not.toContain('c2VjcmV0');
|
||||
expect(line().origin).toBe('https://example.com/cb?[REDACTED]&[REDACTED]&q=[REDACTED]');
|
||||
expect(formatNetworkLogLine({ timestamp: 0, method: 'GET', url: 'https://api.example.com/v1?dG9rZW4=' })).not.toContain('dG9rZW4');
|
||||
});
|
||||
|
||||
it('keeps a URL without a query as it was', () => {
|
||||
write('click', ['@e3'], undefined, 'https://example.com/a/b');
|
||||
expect(line().origin).toBe('https://example.com/a/b');
|
||||
write('click', ['@e3'], undefined, 'about:blank');
|
||||
expect(line().origin).toBe('about:blank');
|
||||
});
|
||||
|
||||
it('cuts an unparseable origin at its query', () => {
|
||||
write('click', ['@e3'], undefined, 'not a url?token=hunter2pw');
|
||||
expect(line().origin).toBe('not a url');
|
||||
});
|
||||
});
|
||||
|
||||
describe('network log line (browse-network.log, GST-1635)', () => {
|
||||
it('redacts query values of page-issued request URLs', () => {
|
||||
const out = formatNetworkLogLine({ timestamp: 0, method: 'GET', url: 'https://api.example.com/v1/me?access_token=tok_abcdef&page=2', status: 200, duration: 5, size: 10 });
|
||||
expect(out).not.toContain('tok_abcdef');
|
||||
expect(out).toBe('[1970-01-01T00:00:00.000Z] GET https://api.example.com/v1/me?access_token=[REDACTED]&page=[REDACTED] → 200 (5ms, 10B)');
|
||||
});
|
||||
|
||||
it('keeps the pending and unknown placeholders', () => {
|
||||
expect(formatNetworkLogLine({ timestamp: 0, method: 'POST', url: 'https://api.example.com/v1/login' }))
|
||||
.toBe('[1970-01-01T00:00:00.000Z] POST https://api.example.com/v1/login → pending (?ms, ?B)');
|
||||
});
|
||||
|
||||
it('is what the server flush writes', () => {
|
||||
const src = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf8');
|
||||
expect(src).toContain('appendSecureFile(NETWORK_LOG_PATH, entries.map(formatNetworkLogLine)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('chain line keeps only real command names (fix-check shapes)', () => {
|
||||
it('an empty quoted selector before an unquoted multi-word value leaves nothing', () => {
|
||||
write('chain', ['fill "" correct horse battery']);
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
- **The browse audit origin and network log no longer keep URL query values or credentials, and short digit pieces leave audit error text.** The `origin` field of `.gstack/browse-audit.jsonl` stored the full page URL, so after a GET form was submitted the next line held what the user typed, such as `?user=a&password=hunter2pw`, and `browse-network.log` stored request URLs with their token query values. In those two places a URL now keeps its scheme, host, path and query parameter names; every query value is written as `[REDACTED]`, a query piece with no value (such as a bare token) is written as `[REDACTED]` whole, and any username, password or `#fragment` is dropped. The audit error scrub now also removes 2- and 3-character pieces of a redacted value that contain a digit, such as a CVV or an expiry month, matched as whole tokens; shorter word pieces like `a` or `is` stay so the error text is readable. Still recorded as typed: navigation command arguments such as a `goto` URL with its query, URL paths, and the text in `browse-console.log`. Lines written before this release still hold the old values; delete `.gstack/browse-audit.jsonl` and `.gstack/browse-network.log` to clear them. (GST-1635)
|
||||
Reference in new issue
Block a user