Skip to content

Commit cb84fd4

Browse files
fix(smart-notes): stop destroy(err) stderr dumps from guarded HTTP GET
The body-limit, timeout, and abort branches passed a typed error to request.destroy(). Under OpenCode's embedded Bun the stream machinery re-surfaces that error as an uncaught stderr dump even with 'error' listeners installed on both the request and the response. Settle the promise with the typed rejection first, then destroy without an error so the internals have nothing to re-emit. The response 'error' listener stays for genuine mid-body transport failures. Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent 1e35f7c commit cb84fd4

1 file changed

Lines changed: 25 additions & 8 deletions

File tree

  • packages/plugin/src/features/magic-context/smart-notes

packages/plugin/src/features/magic-context/smart-notes/ssrf-guard.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,19 +229,30 @@ function requestValidatedAddress(
229229
const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
230230
bytes += buf.byteLength;
231231
if (bytes > options.bodyLimitBytes) {
232-
request.destroy(
232+
// Reject FIRST, then destroy WITHOUT an error argument.
233+
// destroy(err) hands the error to the stream machinery,
234+
// which under OpenCode's embedded Bun has been observed
235+
// re-surfacing it through the readable's flow() as an
236+
// UNCAUGHT stderr dump even with 'error' listeners on
237+
// both the request and the response. An errorless
238+
// destroy gives the internals nothing to re-emit; the
239+
// promise is already settled with the typed error.
240+
reject(
233241
new SmartNoteNetworkError(
234242
"SMART_NOTE_NETWORK: response body too large",
235243
),
236244
);
245+
response.destroy();
246+
request.destroy();
237247
return;
238248
}
239249
chunks.push(buf);
240250
});
241-
// request.destroy(err) (body limit, timeout, abort) also destroys
242-
// this response stream with the same error. Without a listener,
243-
// that becomes an UNCAUGHT stream 'error' dumped to stderr — the
244-
// request-level handler alone does not cover the response side.
251+
// Genuine transport errors mid-body (connection reset, TLS
252+
// failure) surface here. Local aborts (body limit, timeout,
253+
// signal) reject the promise directly and destroy errorless,
254+
// so this listener only sees real network failures — but it
255+
// must exist: an unlistened stream 'error' dumps to stderr.
245256
response.on("error", (error) => {
246257
reject(toNetworkError(error, "response failed"));
247258
});
@@ -260,11 +271,17 @@ function requestValidatedAddress(
260271
},
261272
);
262273

263-
const onAbort = () =>
264-
request.destroy(new SmartNoteNetworkError("SMART_NOTE_NETWORK: aborted"));
274+
// Same reject-then-errorless-destroy discipline as the body-limit
275+
// branch: passing an Error to destroy() lets stream internals re-throw
276+
// it where no listener reaches.
277+
const onAbort = () => {
278+
reject(new SmartNoteNetworkError("SMART_NOTE_NETWORK: aborted"));
279+
request.destroy();
280+
};
265281
options.signal.addEventListener("abort", onAbort, { once: true });
266282
request.on("timeout", () => {
267-
request.destroy(new SmartNoteNetworkError("SMART_NOTE_NETWORK: request timed out"));
283+
reject(new SmartNoteNetworkError("SMART_NOTE_NETWORK: request timed out"));
284+
request.destroy();
268285
});
269286
request.on("error", (error) => {
270287
options.signal.removeEventListener("abort", onAbort);

0 commit comments

Comments
 (0)