starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-17 00:02:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: report extra only when it serializes, replacing the key blocklist

Browse Source 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9xMqWcQBvJwrvMb5Tw3nH
This commit is contained in:
Ruben Fiszel
2026-09-01 09:49:55 +00:00
co-authored by Claude Opus 5
parent d0ec550730
commit 21bba1b762
2 changed files with 39 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/bun_jobs.rs
+22 -26
View File
@@ -401,29 +401,6 @@ async fn test_bun_job_non_error_throws(db: Pool<Postgres>) -> anyhow::Result<()>
assert_eq!(result["error"]["message"], serde_json::json!("null"));
}
// Circular object: several HTTP clients throw errors that link two objects
// both ways, which a plain JSON.stringify cannot serialize.
{
let job = bun_job(
r#"
export function main() {
const a: any = { name: "circular", message: "boom" };
a.self = a;
throw a;
}
"#,
);
let completed = run_job_in_new_worker_until_complete(&db, false, job, port).await;
assert!(!completed.success);
let result = completed.json_result().unwrap();
let error = &result["error"];
assert_eq!(error["message"], serde_json::json!("boom"));
assert_eq!(
error["extra"]["self"]["self"],
serde_json::json!("[Circular]")
);
}
// A thrown string is a string, not an index map of its characters.
{
let job = bun_job(r#"export function main() { throw "nur ein string"; }"#);
@@ -436,8 +413,10 @@ export function main() {
);
}
// Serializing a cycle is what first brings an HTTP client's whole request into
// `extra`, so the transport credentials on it must not reach the stored result.
// An HTTP client links its errors both ways, which is what a plain JSON.stringify
// could not serialize. `extra` is reported only when it serializes on its own, so the
// request the client hangs off the error, credentials included, stays out of the
// result exactly as it did when the wrapper crashed.
{
let job = bun_job(
r#"
@@ -446,6 +425,7 @@ export function main() {
url: "/x",
headers: { Authorization: "Bearer sentinel-tkn" },
auth: { username: "svc", password: "sentinel-tkn" },
httpsAgent: { options: { key: "-----BEGIN PRIVATE KEY-----sentinel-tkn" } },
};
const request: any = { path: "/x", _header: "GET /x HTTP/1.1\r\nAuthorization: Bearer sentinel-tkn\r\n\r\n" };
const response: any = { status: 401, config, request };
@@ -467,13 +447,29 @@ export function main() {
error["message"],
serde_json::json!("Request failed with status code 401")
);
assert_eq!(error["extra"]["response"]["status"], serde_json::json!(401));
assert_eq!(error["name"], serde_json::json!("AxiosError"));
assert_eq!(
error["extra"],
serde_json::json!("[extra omitted: not serializable]")
);
assert!(
!error.to_string().contains("sentinel-tkn"),
"credential reached the result: {error}"
);
}
// An acyclic thrown object still reports its own properties, unchanged.
{
let job = bun_job(r#"export function main() { throw { code: 42, hint: "kein Error" }; }"#);
let completed = run_job_in_new_worker_until_complete(&db, false, job, port).await;
assert!(!completed.success);
let result = completed.json_result().unwrap();
assert_eq!(
result["error"]["extra"],
serde_json::json!({ "code": 42, "hint": "kein Error" })
);
}
Ok(())
}
backend/windmill-worker/src/common.rs
+17 -12
View File
@@ -79,20 +79,19 @@ pub const DEV_CONF_NSJAIL: &str = "";
/// Enabling it there would newly persist whatever an HTTP client hangs off its errors,
/// credential-bearing headers included, into existing scripts' run history.
///
/// `wmErrDescribe` renders a thrown object's contents, so it stays behind the same line:
/// it runs only for a value exposing none of message/name/stack, which an HTTP client's
/// error never is, and only to fill a message that would otherwise be `[object Object]`.
/// `extra` is reported only when it serializes on its own, which is exactly when the
/// pre-existing bare `JSON.stringify` would have succeeded. Anything else is what used to
/// crash, so persisting it now would be new exposure: an HTTP client hangs its whole
/// request off a cyclic error, credentials and mTLS keys included, and no blocklist of key
/// names can be complete. Keeping the condition rather than the key list is what bounds it.
///
/// Making a cyclic graph serializable is what puts an HTTP client's whole request under
/// `extra` for the first time, so the replacer drops transport-credential keys and the
/// walk is capped: `console.error` prints those nested objects as `[Object ...]`, and the
/// crash kept them out of the result entirely, so neither had reached a stored result.
/// `wmErrDescribe` also renders a thrown object's contents, so it stays behind the same
/// line: it fills a message only for a value that yielded no field at all, never for one
/// carrying `message`/`name`/`stack` or reportable `extra`.
///
/// Comment-free on purpose: it is written into each job's wrapper source.
pub const JS_ERROR_SERIALIZER: &str = r#"
const wmErrOwnKeys = ['line', 'name', 'stack', 'column', 'message', 'sourceURL', 'originalLine', 'originalColumn'];
const wmErrSecretKeys = /^(authorization|proxy-authorization|cookie|set-cookie|x-api-key|api[-_]?key|auth|passphrase|_header)$/i;
const wmErrMaxExtra = 100000;
function wmErrString(v) {
if (typeof v === 'string') return v;
@@ -111,6 +110,14 @@ function wmErrProp(e, key) {
}
}
function wmErrSerializable(v) {
try {
return JSON.stringify(v) !== undefined;
} catch (_) {
return false;
}
}
function wmErrDescribe(v) {
if (v === null || typeof v !== 'object') return wmErrString(v);
let s;
@@ -142,8 +149,7 @@ function wmToErrorObject(e, collectExtra) {
if (v !== undefined) extra[key] = v;
}
if (Object.keys(extra).length > 0) {
const s = wmSerializeError(extra);
err.extra = s !== undefined && s.length <= wmErrMaxExtra ? extra : '[extra omitted: too large]';
err.extra = wmErrSerializable(extra) ? extra : '[extra omitted: not serializable]';
}
}
if (Object.keys(err).length === 0) {
@@ -157,7 +163,6 @@ function wmSerializeError(err) {
const ancestors = [];
try {
return JSON.stringify(err, function (key, v) {
if (wmErrSecretKeys.test(key)) return '[redacted]';
if (v === null || typeof v !== 'object') return v;
while (ancestors.length > 0 && ancestors[ancestors.length - 1] !== this) ancestors.pop();
if (ancestors.indexOf(v) !== -1) return '[Circular]';