Skip to content

Commit da95565

Browse files
committed
fix(audit): harden native lab gates and persistence
1 parent 17a8b4a commit da95565

11 files changed

Lines changed: 295 additions & 31 deletions

docs/audits/AUDIT_2026-06-06.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Audit 2026-06-06
2+
3+
Scope: v2.0.135 baseline, open GitHub issues, VPS deployment state, native bridge protocol notes, release workflow, dashboard performance, and HTTP ingress behavior.
4+
5+
## Baseline
6+
7+
- Local HEAD before this audit: v2.0.135 (`17a8b4a`), working tree clean.
8+
- VPS was already running v2.0.135 and `/health` reported the commit after the prior deployment fix.
9+
- Open issue clusters: upstream provider deadline around 236-243s, tool-call stability, WebSearch/WebFetch protocol work, SWE-1.6 special-agent routing, and dashboard large-list performance.
10+
- One security subagent failed with a remote 429, so security findings here are from the main-thread read-through plus the successful protocol/performance/release subagents.
11+
12+
## Findings
13+
14+
### High: release metadata was not injected into GHCR images
15+
16+
`Dockerfile` accepts `BUILD_VERSION`, `BUILD_COMMIT`, `BUILD_COMMIT_MESSAGE`, `BUILD_COMMIT_DATE`, and `BUILD_BRANCH`, and `/health` reads the exported `WINDSURFAPI_BUILD_*` env. The tag release workflow did not pass those build args. Because `.dockerignore` excludes `.git`, release containers had no reliable source for `/health.commit`; Docker labels could still contain a revision, but `/health` could not read labels.
17+
18+
Status in this audit: blocked by the current GitHub token, which has `repo` but not `workflow` scope. GitHub rejects any push that modifies `.github/workflows/release.yml` without `workflow` scope. Keep this as the first follow-up once a workflow-scoped token is available.
19+
20+
### High: WebFetch/Read native bridge is still lab-only
21+
22+
Confirmed facts:
23+
24+
- Native bridge defaults remain narrow. Read/WebFetch/WebSearch are mapped but not in the default mature native set.
25+
- WebFetch direct API that returns URL content has not been found. The observed official path is LS `HandleCascadeUserInteraction` for `read_url_content` permission approval, followed by trajectory steps.
26+
- `read_url_content` completed output should come from `web_document` field 2. The top-level field 5 fallback was historical and is not confirmed for current official protocol.
27+
- Read wrapper `type=14 field=19` remains partially reversed. v2.0.131 stopped prompt text from being promoted as a path, but that does not make Read production-ready.
28+
29+
Fix in this audit: WebFetch auto-approve now canonicalizes URL/origin and only accepts http(s) URLs without embedded credentials. `read_url_content` top-level field 5 fallback is now behind `WINDSURFAPI_NATIVE_TOOL_BRIDGE_READ_URL_LEGACY_SUMMARY=1`; default parsing only trusts `web_document`.
30+
31+
Still not fixed by design: do not production-open Read/WebSearch/WebFetch by default. Continue gated canaries only.
32+
33+
### Medium: oversized HTTP bodies were misclassified
34+
35+
`readBody()` detected bodies over 10 MB, but route-level JSON parse catches returned `400 Invalid JSON` for chat/responses/messages. That misled clients and operators.
36+
37+
Fix in this audit: `readBody()` now preserves a 413-class error and routes return protocol-shaped 413 payloads.
38+
39+
### Medium: atomic JSON temp files could race across processes
40+
41+
Several JSON persisters used one fixed sibling temp path such as `accounts.json.tmp` or `config.json.tmp`. Concurrent writers in separate processes could collide on the same temp file; on Windows, replacing the final target can also briefly fail while another test/process has the file open.
42+
43+
Fix in this audit: atomic JSON writers now use unique sibling temp names and retry short-lived `EPERM`/`EBUSY` rename failures before surfacing the error.
44+
45+
### Medium: dashboard account pages do full heavy-list rendering
46+
47+
`GET /dashboard/api/accounts` returns all accounts with heavy fields such as capabilities, credits, available models, tier models, user status, and LS admission. The front end renders all account rows at once; the anomaly page also full-renders and polls every 30 seconds. This matches issue #168.
48+
49+
Not fixed in this audit: this needs a separate dashboard pagination/summary endpoint slice. It is larger than the release/protocol hardening changes and should not be mixed into the same patch.
50+
51+
### Medium: stream and cache memory pressure remain bounded by count, not bytes
52+
53+
Conversation pool and response cache have item limits and TTLs. Response cache does not have a byte cap, and streaming accumulates text for downstream usage/cache logic. SSE writes do not wait for `drain` on slow clients.
54+
55+
Not fixed in this audit: recommended follow-up is byte caps plus backpressure-aware SSE writes.
56+
57+
## Non-Bugs / External Limits
58+
59+
- The 236-243s `context deadline exceeded` issue is a Windsurf upstream provider/Cascade stream window. Local `CASCADE_MAX_WAIT_MS` is not the limiting factor.
60+
- SWE-1.6 should not be treated as a normal catalog or entitlement bug. It should continue through the Devin/ACP special-agent POC path.
61+
- Trial/free rate limits, entitlement misses, and IP-level cooldowns are upstream capacity controls; local routing can surface them clearly but cannot create upstream quota.
62+
63+
## Next Execution Plan
64+
65+
1. Release and deploy the current hardening patch, then smoke VPS.
66+
2. With a token that has `workflow` scope, apply the release workflow metadata/test-gate fix.
67+
3. Next patch: dashboard account pagination/lightweight summary endpoint for #168.
68+
4. Next protocol work: continue raw-child traces for `type=14 field=19` Read wrapper and WebSearch/WebFetch traces, but keep production native defaults narrow.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# v2.0.136 - audit hardening
2+
3+
## What changed
4+
5+
- Oversized HTTP request bodies now return protocol-shaped `413 Request body too large` errors instead of being misclassified as `Invalid JSON`.
6+
- WebFetch lab auto-approve now canonicalizes URL/origin allowlist entries and rejects non-http(s), malformed, or credential-bearing URLs.
7+
- `read_url_content` no longer trusts the unconfirmed top-level field 5 summary by default; legacy fallback requires `WINDSURFAPI_NATIVE_TOOL_BRIDGE_READ_URL_LEGACY_SUMMARY=1`.
8+
- Atomic JSON writes now use unique temporary filenames and retry short-lived Windows rename locks, including `accounts.json`.
9+
- Added `docs/audits/AUDIT_2026-06-06.md` with the audit baseline, findings, fixed items, and remaining follow-ups.
10+
11+
## Notes
12+
13+
- Read/WebSearch/WebFetch remain lab-only native bridge surfaces. This release tightens canary behavior; it does not production-open those tools.
14+
- Release workflow metadata/test-gate hardening is documented in the audit, but was not included in this release because the current GitHub token lacks `workflow` scope.
15+
- Dashboard account pagination for #168 is the next recommended implementation slice.
16+
17+
## Validation
18+
19+
- `node --test test/*.test.js` -> 1070/1070 passing.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "windsurf-api",
3-
"version": "2.0.135",
3+
"version": "2.0.136",
44
"description": "Windsurf to OpenAI + Anthropic compatible API proxy. Turns Windsurf's 107 AI models (Claude, GPT, Gemini, DeepSeek, Grok, Qwen, Kimi, GLM, SWE) into dual-protocol API endpoints. Zero npm deps.",
55
"type": "module",
66
"main": "src/index.js",

src/auth.js

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@
1313
import { createHash, randomUUID, timingSafeEqual } from 'crypto';
1414
import { isStickyEnabled, getStickyBinding, setStickyBinding, clearStickyBinding } from './account/sticky-session.js';
1515
import { isExperimentalEnabled } from './runtime-config.js';
16-
import { readFileSync, writeFileSync, existsSync, renameSync, unlinkSync, readdirSync } from 'fs';
16+
import { readFileSync, writeFileSync, existsSync, unlinkSync, readdirSync } from 'fs';
1717
import { config, log } from './config.js';
18+
import { renameSyncWithRetry } from './fs-atomic.js';
1819
import { getEffectiveProxy } from './dashboard/proxy-config.js';
1920
import { getTierModels, getModelKeysByEnum, MODELS, registerDiscoveredFreeModel } from './models.js';
2021
import { getLsAdmissionStatus, getLsMaintenanceRequests } from './langserver.js';
@@ -247,13 +248,14 @@ function _serializeAccounts() {
247248
function saveAccounts() {
248249
if (_saveInFlight) { _savePending = true; return; }
249250
_saveInFlight = true;
250-
const tempFile = ACCOUNTS_FILE + '.tmp';
251+
const tempFile = `${ACCOUNTS_FILE}.${process.pid}.${randomUUID().slice(0, 8)}.tmp`;
251252
try {
252-
// Atomic write: write to .tmp then rename so a crash mid-write can't
253-
// leave accounts.json truncated/corrupt. Node's renameSync is atomic
254-
// on POSIX and replaces the target on Windows (fs.rename behavior).
253+
// Atomic write: write to a unique sibling tmp then rename so a crash
254+
// mid-write cannot leave accounts.json truncated/corrupt. The unique
255+
// tmp also prevents concurrent test/process saves from racing on the
256+
// same `${ACCOUNTS_FILE}.tmp` name.
255257
writeFileSync(tempFile, JSON.stringify(_serializeAccounts(), null, 2));
256-
renameSync(tempFile, ACCOUNTS_FILE);
258+
renameSyncWithRetry(tempFile, ACCOUNTS_FILE);
257259
} catch (e) {
258260
log.error('Failed to save accounts:', e.message);
259261
try { unlinkSync(tempFile); } catch {}
@@ -271,10 +273,10 @@ function saveAccounts() {
271273
* atomic.
272274
*/
273275
export function saveAccountsSync() {
274-
const tempFile = ACCOUNTS_FILE + '.shutdown.tmp';
276+
const tempFile = `${ACCOUNTS_FILE}.${process.pid}.shutdown.tmp`;
275277
try {
276278
writeFileSync(tempFile, JSON.stringify(_serializeAccounts(), null, 2));
277-
renameSync(tempFile, ACCOUNTS_FILE);
279+
renameSyncWithRetry(tempFile, ACCOUNTS_FILE);
278280
} catch (e) {
279281
log.error('Shutdown: failed to flush accounts:', e.message);
280282
try { unlinkSync(tempFile); } catch {}
@@ -318,7 +320,7 @@ export function migrateReplicaAccountsTo({ sharedDir, accountsFile, logger = log
318320
const tempFile = accountsFile + '.migrate.tmp';
319321
try {
320322
writeFileSync(tempFile, JSON.stringify([...merged.values()], null, 2));
321-
renameSync(tempFile, accountsFile);
323+
renameSyncWithRetry(tempFile, accountsFile);
322324
logger.warn?.(`Migrated ${merged.size} account(s) from ${scanned} replica-* subdir(s) into ${accountsFile} (issue #67)`);
323325
return { migrated: merged.size, scanned, skipped: false };
324326
} catch (e) {

src/client.js

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,38 @@ function csvSetEnv(name) {
7373
.filter(Boolean));
7474
}
7575

76-
function isReadUrlAutoApproveAllowed(url, origin) {
76+
function canonicalWebUrl(value) {
77+
const raw = String(value || '').trim();
78+
if (!raw) return null;
79+
let parsed;
80+
try { parsed = new URL(raw); } catch { return null; }
81+
if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') return null;
82+
if (parsed.username || parsed.password) return null;
83+
parsed.hash = '';
84+
return {
85+
origin: parsed.origin,
86+
href: parsed.href.replace(/\/+$/, ''),
87+
};
88+
}
89+
90+
export function isReadUrlAutoApproveAllowed(url, origin) {
7791
if (process.env.WINDSURFAPI_NATIVE_TOOL_BRIDGE_WEBFETCH_AUTO_APPROVE !== '1') return false;
7892
const allow = csvSetEnv('WINDSURFAPI_NATIVE_TOOL_BRIDGE_WEBFETCH_AUTO_APPROVE_ORIGINS');
7993
if (!allow.size) return false;
80-
const candidates = [origin, url].map(v => String(v || '').trim()).filter(Boolean);
81-
return candidates.some(v => allow.has(v) || allow.has(v.replace(/\/+$/, '')));
94+
const requestUrl = canonicalWebUrl(url);
95+
if (!requestUrl) return false;
96+
const originHint = canonicalWebUrl(origin)?.origin || '';
97+
if (originHint && originHint !== requestUrl.origin) return false;
98+
for (const entry of allow) {
99+
const allowed = canonicalWebUrl(entry);
100+
if (!allowed) continue;
101+
if (allowed.href === allowed.origin) {
102+
if (allowed.origin === requestUrl.origin) return true;
103+
continue;
104+
}
105+
if (allowed.href === requestUrl.href) return true;
106+
}
107+
return false;
82108
}
83109

84110
function isImageLikeBlock(part) {

src/fs-atomic.js

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
* silently falls back to defaults — user loses every persisted
1010
* setting, model-access list, proxy config, etc.
1111
*
12-
* Pattern: write the new contents to a sibling `${target}.tmp` first,
12+
* Pattern: write the new contents to a unique sibling `${target}.*.tmp` first,
1313
* then `rename(2)` it onto the target. rename is atomic on POSIX and
1414
* replaces an existing target on Windows (per Node's documented
1515
* fs.renameSync behavior). A crash between writeFileSync(tmp) and
@@ -25,13 +25,35 @@
2525
* because it has its own coalescing/_saveInFlight machinery).
2626
*/
2727

28+
import { randomUUID } from 'node:crypto';
2829
import { writeFileSync, renameSync, unlinkSync } from 'node:fs';
2930

31+
const RETRYABLE_RENAME_CODES = new Set(['EPERM', 'EBUSY']);
32+
33+
function sleepSync(ms) {
34+
try {
35+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
36+
} catch {}
37+
}
38+
39+
export function renameSyncWithRetry(sourcePath, targetPath, { attempts = 6, baseDelayMs = 10 } = {}) {
40+
const maxAttempts = Math.max(1, attempts | 0);
41+
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
42+
try {
43+
renameSync(sourcePath, targetPath);
44+
return;
45+
} catch (err) {
46+
if (!RETRYABLE_RENAME_CODES.has(err?.code) || attempt === maxAttempts) throw err;
47+
sleepSync(baseDelayMs * attempt);
48+
}
49+
}
50+
}
51+
3052
export function writeJsonAtomic(targetPath, value, { spaces = 2 } = {}) {
31-
const tmp = `${targetPath}.tmp`;
53+
const tmp = `${targetPath}.${process.pid}.${randomUUID().slice(0, 8)}.tmp`;
3254
try {
3355
writeFileSync(tmp, JSON.stringify(value, null, spaces));
34-
renameSync(tmp, targetPath);
56+
renameSyncWithRetry(tmp, targetPath);
3557
} catch (err) {
3658
try { unlinkSync(tmp); } catch {}
3759
throw err;

src/server.js

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,60 @@ const VERSION_INFO = getVersionInfo();
3838

3939
// 10 MB is way above any realistic chat-completions payload while still
4040
// bounding worst-case memory from a malicious/broken client.
41-
const MAX_BODY_SIZE = 10 * 1024 * 1024;
41+
export const MAX_BODY_SIZE = 10 * 1024 * 1024;
4242

43-
function readBody(req) {
43+
export function readBody(req) {
4444
return new Promise((resolve, reject) => {
4545
const chunks = [];
4646
let size = 0;
47+
let settled = false;
48+
const fail = (err) => {
49+
if (settled) return;
50+
settled = true;
51+
chunks.length = 0;
52+
try { req.resume?.(); } catch {}
53+
reject(err);
54+
};
4755
req.on('data', c => {
56+
if (settled) return;
4857
size += c.length;
4958
if (size > MAX_BODY_SIZE) {
50-
req.destroy();
51-
reject(Object.assign(new Error('Request body too large'), { statusCode: 413 }));
59+
fail(Object.assign(new Error('Request body too large'), {
60+
statusCode: 413,
61+
code: 'ERR_REQUEST_BODY_TOO_LARGE',
62+
}));
5263
return;
5364
}
5465
chunks.push(c);
5566
});
56-
req.on('end', () => resolve(Buffer.concat(chunks).toString('utf-8')));
57-
req.on('error', reject);
67+
req.on('end', () => {
68+
if (settled) return;
69+
settled = true;
70+
resolve(Buffer.concat(chunks).toString('utf-8'));
71+
});
72+
req.on('error', fail);
5873
});
5974
}
6075

76+
export function bodyTooLargePayload(style = 'openai') {
77+
if (style === 'dashboard') {
78+
return { ok: false, error: 'ERR_REQUEST_BODY_TOO_LARGE', message: 'Request body too large' };
79+
}
80+
if (style === 'legacy') {
81+
return { error: 'Request body too large' };
82+
}
83+
if (style === 'anthropic') {
84+
return { type: 'error', error: { type: 'invalid_request_error', message: 'Request body too large' } };
85+
}
86+
return { error: { message: 'Request body too large', type: 'invalid_request' } };
87+
}
88+
89+
function sendBodyTooLargeIfNeeded(res, err, style = 'openai') {
90+
if (err?.statusCode !== 413 && err?.code !== 'ERR_REQUEST_BODY_TOO_LARGE') return false;
91+
json(res, 413, bodyTooLargePayload(style));
92+
return true;
93+
}
94+
6195
export function extractToken(req) {
6296
// Anthropic SDK + OAI SDK compatibility: accept either header.
6397
const authHeader = String(req.headers['authorization'] || '').trim();
@@ -176,7 +210,9 @@ async function route(req, res) {
176210
if (path.startsWith('/dashboard/api/')) {
177211
let body = {};
178212
if (method === 'POST' || method === 'PUT' || method === 'PATCH') {
179-
try { body = JSON.parse(await readBody(req)); } catch {}
213+
try { body = JSON.parse(await readBody(req)); } catch (err) {
214+
if (sendBodyTooLargeIfNeeded(res, err, 'dashboard')) return;
215+
}
180216
}
181217
const subpath = path.slice('/dashboard/api'.length);
182218
return handleDashboardApi(method, subpath, body, req, res);
@@ -251,7 +287,8 @@ async function route(req, res) {
251287

252288
if (path === '/auth/login' && method === 'POST') {
253289
let body;
254-
try { body = JSON.parse(await readBody(req)); } catch {
290+
try { body = JSON.parse(await readBody(req)); } catch (err) {
291+
if (sendBodyTooLargeIfNeeded(res, err, 'legacy')) return;
255292
return json(res, 400, { error: 'Invalid JSON' });
256293
}
257294

@@ -334,7 +371,8 @@ async function route(req, res) {
334371
}
335372

336373
let body;
337-
try { body = JSON.parse(await readBody(req)); } catch {
374+
try { body = JSON.parse(await readBody(req)); } catch (err) {
375+
if (sendBodyTooLargeIfNeeded(res, err, 'openai')) return;
338376
return json(res, 400, { error: { message: 'Invalid JSON', type: 'invalid_request' } });
339377
}
340378
if (!Array.isArray(body.messages)) {
@@ -392,7 +430,8 @@ async function route(req, res) {
392430
}
393431

394432
let body;
395-
try { body = JSON.parse(await readBody(req)); } catch {
433+
try { body = JSON.parse(await readBody(req)); } catch (err) {
434+
if (sendBodyTooLargeIfNeeded(res, err, 'openai')) return;
396435
return json(res, 400, { error: { message: 'Invalid JSON', type: 'invalid_request' } });
397436
}
398437
if (body.input == null) {
@@ -435,7 +474,8 @@ async function route(req, res) {
435474
return json(res, 503, { type: 'error', error: { type: 'api_error', message: 'No active accounts' } });
436475
}
437476
let body;
438-
try { body = JSON.parse(await readBody(req)); } catch {
477+
try { body = JSON.parse(await readBody(req)); } catch (err) {
478+
if (sendBodyTooLargeIfNeeded(res, err, 'anthropic')) return;
439479
return json(res, 400, { type: 'error', error: { type: 'invalid_request_error', message: 'Invalid JSON' } });
440480
}
441481
if (!Array.isArray(body.messages) || body.messages.length === 0) {

src/windsurf.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ import {
6464
} from './proto.js';
6565
import { getSystemPrompts } from './runtime-config.js';
6666

67+
function readUrlLegacySummaryFallbackEnabled() {
68+
return process.env.WINDSURFAPI_NATIVE_TOOL_BRIDGE_READ_URL_LEGACY_SUMMARY === '1';
69+
}
70+
6771
// ─── Enums ─────────────────────────────────────────────────
6872

6973
export const SOURCE = {
@@ -1261,7 +1265,9 @@ export function parseTrajectorySteps(buf) {
12611265
argumentsJson = JSON.stringify(args);
12621266
const webDocument = getField(body, 2, 2);
12631267
result = webDocument ? decodeKnowledgeBaseItemText(webDocument.value) : '';
1264-
if (!result) result = getField(body, 5, 2)?.value?.toString('utf8') || '';
1268+
if (!result && readUrlLegacySummaryFallbackEnabled()) {
1269+
result = getField(body, 5, 2)?.value?.toString('utf8') || '';
1270+
}
12651271
if (!result && readUrlRequestedInteraction) continue;
12661272
} else if (kind === 'search_web') {
12671273
const args = {

test/audit-fixes.test.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,10 @@ describe('writeJsonAtomic (audit fix #1: durable config writes)', () => {
3333
const target = join(dir, 'config.json');
3434
writeJsonAtomic(target, { hello: 'world', n: 1 });
3535
assert.deepEqual(JSON.parse(readFileSync(target, 'utf8')), { hello: 'world', n: 1 });
36-
// The .tmp sibling must NOT exist after a successful write —
36+
// Tmp siblings must NOT exist after a successful write —
3737
// otherwise we'd leak garbage into DATA_DIR every save.
38-
assert.ok(!existsSync(`${target}.tmp`), '.tmp file should be removed after rename');
38+
const leftover = readdirSync(dir).filter(f => f.endsWith('.tmp'));
39+
assert.deepEqual(leftover, [], `expected no .tmp leftovers, got ${leftover.join(',')}`);
3940
} finally {
4041
rmSync(dir, { recursive: true, force: true });
4142
}

0 commit comments

Comments
 (0)