You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Per the Contributing guide, I'm sharing this here rather than opening pull requests. I recently did a deep security-focused review of DeepSeek Harness — attacker-style passes over the web capability seam, sandbox roots, filesystem tools, the tools runtime, and the host webserver — verifying every finding against source before counting it. Seven survived. Each is small to fix, and I've put working, tested reference implementations on my fork (johalputt/deepseek-harness, master) in case any are useful — sharing them purely as ecosystem reference, since I know external PRs aren't accepted right now.
1. SSRF: dsh-web-fetch-http has no destination restriction
The provider validates URL shape (scheme, credentials, length) but nothing about where the hostname resolves. A model-supplied URL can point harness egress at loopback services, LAN hosts, or cloud metadata (169.254.169.254). The capability-seam note correctly lists this as deferred top priority.
What closes it: per-hop resolve-then-validate refusing non-public resolutions before any socket opens, plus connect-time re-validation inside the dispatcher (undici Agent({connect:{lookup}})) so DNS rebinding can't slip through between check and connect. The classifier needs IPv4, native IPv6, and embedded-IPv4 forms (::ffff:-mapped, NAT64 64:ff9b::/96, 6to4 2002::/16) — a v4-only list misses all three. One subtlety tests caught: a dotted tail must parse only as the final component, or 10.0.0.1:: classifies as an unrelated public v6 address. Suggests a new error code like WEB_PRIVATE_NETWORK, default-on config with an opt-out for deployments targeting loopback services.
2. Harness-served pages are framable (clickjacking)
Framed loads pass the /api trust fence by design — the embedding page never needs harness credentials — which leaves clickjacking as the one cross-site path reaching approval UIs and session content through a legitimate origin. Two headers at the single response choke point fix it for routes, fallbacks, and upgrades alike: x-frame-options: DENY + CSP frame-ancestors 'none'.
3. Glob results can be forged by filenames containing \n
ripgrep frames records with newlines by default, and POSIX filenames may contain newlines. A crafted name like x\n/etc/passwd splits into two result paths — fabricating a discovered file out of attacker-chosen text. Passing -0 and splitting records on NUL (which filenames cannot contain) makes records unambiguous.
4. Oversized timeoutMs silently becomes a 1 ms deadline
Node coerces timer delays above 2147483647 ms down to 1 ms, so a config-derived timeoutMs over the timer ceiling doesn't fail — it makes every execution of that tool time out instantly. Rejecting it at defineTool()/register time follows the repo's fail-loud-at-load rule.
5. Sandbox grants a phantom C:\tmp writable root on Windows
Node re-roots literal /tmp to <drive>:\tmp on win32, so the POSIX shared writable root silently becomes a nonexistent, colliding directory in writableRoots(). Granting /tmp only on POSIX keeps the grant meaningful.
6. No standing vulnerability/secret scanning
Tests prove behavior but can't catch a CVE published against an already-merged dependency or a secret that entered history. A weekly CodeQL + gitleaks + pnpm audit --prod --audit-level high workflow catches all three classes. One lesson from setting this up: audit floors in pnpm-workspace.yaml overrides must be exact versions — open-ended floors (>=x.y.z) float to newer majors wherever a flow resolves freshly, which broke our release packing until pinned.
7. CI observation: Sandbox darwin-parity lane fails on current hosted macOS runners
sandbox.yml's seatbelt/macos job fails on macos-latest with three assertion errors (subprocess cwd clipping in output, PTY state reporting inferred_idle where stdin_read is expected, and a [Console]::OutputEncoding utf-8 assertion). Reproduced on two independent runner images — looks like hosted-image drift rather than product regression, but flagging since it may hide real signal for you too.
All seven have test coverage on the fork (classifier tables, zero-connection-when-blocked proven against live loopback servers, framing regressions, platform gates); typecheck and lint clean throughout. Happy to split this into separate discussions per item if that's easier to triage and upvote, go deeper on any mechanism, or share patch references for whatever's useful. Thanks for building this in the open — the Agent Notes made the review dramatically easier.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
Hi team 👋
Per the Contributing guide, I'm sharing this here rather than opening pull requests. I recently did a deep security-focused review of DeepSeek Harness — attacker-style passes over the web capability seam, sandbox roots, filesystem tools, the tools runtime, and the host webserver — verifying every finding against source before counting it. Seven survived. Each is small to fix, and I've put working, tested reference implementations on my fork (
johalputt/deepseek-harness,master) in case any are useful — sharing them purely as ecosystem reference, since I know external PRs aren't accepted right now.1. SSRF:
dsh-web-fetch-httphas no destination restrictionThe provider validates URL shape (scheme, credentials, length) but nothing about where the hostname resolves. A model-supplied URL can point harness egress at loopback services, LAN hosts, or cloud metadata (
169.254.169.254). The capability-seam note correctly lists this as deferred top priority.What closes it: per-hop resolve-then-validate refusing non-public resolutions before any socket opens, plus connect-time re-validation inside the dispatcher (undici
Agent({connect:{lookup}})) so DNS rebinding can't slip through between check and connect. The classifier needs IPv4, native IPv6, and embedded-IPv4 forms (::ffff:-mapped, NAT6464:ff9b::/96, 6to42002::/16) — a v4-only list misses all three. One subtlety tests caught: a dotted tail must parse only as the final component, or10.0.0.1::classifies as an unrelated public v6 address. Suggests a new error code likeWEB_PRIVATE_NETWORK, default-on config with an opt-out for deployments targeting loopback services.2. Harness-served pages are framable (clickjacking)
Framed loads pass the
/apitrust fence by design — the embedding page never needs harness credentials — which leaves clickjacking as the one cross-site path reaching approval UIs and session content through a legitimate origin. Two headers at the single response choke point fix it for routes, fallbacks, and upgrades alike:x-frame-options: DENY+ CSPframe-ancestors 'none'.3. Glob results can be forged by filenames containing
\nripgrep frames records with newlines by default, and POSIX filenames may contain newlines. A crafted name like
x\n/etc/passwdsplits into two result paths — fabricating a discovered file out of attacker-chosen text. Passing-0and splitting records on NUL (which filenames cannot contain) makes records unambiguous.4. Oversized
timeoutMssilently becomes a 1 ms deadlineNode coerces timer delays above 2147483647 ms down to 1 ms, so a config-derived
timeoutMsover the timer ceiling doesn't fail — it makes every execution of that tool time out instantly. Rejecting it atdefineTool()/register time follows the repo's fail-loud-at-load rule.5. Sandbox grants a phantom
C:\tmpwritable root on WindowsNode re-roots literal
/tmpto<drive>:\tmpon win32, so the POSIX shared writable root silently becomes a nonexistent, colliding directory inwritableRoots(). Granting/tmponly on POSIX keeps the grant meaningful.6. No standing vulnerability/secret scanning
Tests prove behavior but can't catch a CVE published against an already-merged dependency or a secret that entered history. A weekly CodeQL + gitleaks +
pnpm audit --prod --audit-level highworkflow catches all three classes. One lesson from setting this up: audit floors inpnpm-workspace.yamloverrides must be exact versions — open-ended floors (>=x.y.z) float to newer majors wherever a flow resolves freshly, which broke our release packing until pinned.7. CI observation: Sandbox darwin-parity lane fails on current hosted macOS runners
sandbox.yml's seatbelt/macos job fails onmacos-latestwith three assertion errors (subprocess cwd clipping in output, PTY state reportinginferred_idlewherestdin_readis expected, and a[Console]::OutputEncodingutf-8 assertion). Reproduced on two independent runner images — looks like hosted-image drift rather than product regression, but flagging since it may hide real signal for you too.All seven have test coverage on the fork (classifier tables, zero-connection-when-blocked proven against live loopback servers, framing regressions, platform gates); typecheck and lint clean throughout. Happy to split this into separate discussions per item if that's easier to triage and upvote, go deeper on any mechanism, or share patch references for whatever's useful. Thanks for building this in the open — the Agent Notes made the review dramatically easier.
All reactions