fix(security): reject non-loopback Host headers in viewer server (DNS rebinding, CWE-350)

[aaronjmars]

Disclosure path

Patch branch has been carried privately since 2026-05-11 while I tried the GHSA route — gh api /repos/rohitg00/agentmemory/security-advisories returned 403 because PVR is disabled at the repo level. Filing publicly so the fix lands; happy to fold this into a GHSA the moment you flip PVR on. Coordinated with operator before going public.

Suggested CVSS 3.1: 7.5 — AV:N/AC:H/PR:N/UI:R/S:U/C:H/I:H/A:N. CWE-350 primary (CWE-352 on the write subset).

Summary

The viewer (default :3113) does not validate the HTTP Host header. A DNS-rebinding attacker can lure a victim to attacker.com, short-TTL-rebind the hostname to 127.0.0.1, and read every memory the user has stored — full bypass of the VIEWER_ALLOWED_ORIGINS CORS allowlist because the request is, from the browser's perspective, same-origin.

Impact

src/viewer/server.ts binds to 127.0.0.1 and proxies every request to the local REST API with the AGENTMEMORY_SECRET attached as a Bearer token. The Origin allowlist works against a plain cross-origin attacker page — fetch to localhost:3113 from attacker.com triggers a CORS check, server reflects Access-Control-Allow-Origin: http://localhost:3111 (the fallback), browser blocks the JS read. Preflight blocks state-changing non-simple methods the same way.

DNS rebinding sidesteps the defence because the browser sees same-origin on both sides:

1. Attacker registers attacker.com, runs an HTTP server on :3113 with a public IP and short TTL.
2. Victim browses http://attacker.com:3113. Page loads, JS waits for the DNS cache TTL to expire (60–120 s in modern browsers).
3. Attacker DNS now returns 127.0.0.1. Next lookup for attacker.com lands on loopback.
4. JS fires fetch('/agentmemory/export'). URL = http://attacker.com:3113/agentmemory/export — same origin as the page that ran the script. CORS is not invoked.
5. TCP connection lands on 127.0.0.1:3113 (the viewer). Host: attacker.com:3113. The viewer never looks at Host, proxies the request to 127.0.0.1:3111/agentmemory/export with the bearer attached, returns the body.
6. Attacker's JS reads the full memory export and exfiltrates it.

Same primitive lands on every endpoint the viewer proxies — reads (/export, /memories, /audit, /observations, /sessions, /snapshots) and writes (/remember, /forget, /evict, /import, /governance/memories, /snapshot/restore) — with the user's full secret. The "private memory" guarantee in README.md is broken end-to-end.

Proof (local repro — no DNS gymnastics needed)

# Terminal 1
npm start            # wait for `[agentmemory] Viewer: http://localhost:3113`

# Terminal 2
curl -i -H 'Host: attacker.com:3113' http://127.0.0.1:3113/agentmemory/livez
# 200 OK — request is processed; in a real rebind flow the JS reads the body.

curl -s -H 'Host: attacker.com:3113' http://127.0.0.1:3113/agentmemory/export | head -c 200
# Returns the export JSON. With AGENTMEMORY_SECRET configured, the viewer
# adds the bearer for you — the response is fully authenticated.

I am not attaching a working DNS-rebind chain per disclosure hygiene; the curl repro is the primitive, and the rest is standard rebind.network / dheriot-style payload (already used against etcd, Kubelet, Plex, Home Assistant, etc.).

Fix shape

• Derive a Host-header allowlist from VIEWER_ALLOWED_ORIGINS plus the viewer's actual listen port (resolved post-listen() so port=0 unit tests work) plus a VIEWER_ALLOWED_HOSTS override env var for the rare reverse-proxy case.
• Reject any other Host with 403 forbidden host before the route logic runs (HTML branch and proxy branch).
• buildAllowedHosts and isHostAllowed exported as pure functions.
• 13 new tests in test/viewer-security.test.ts:
  • 8 predicate tests (loopback accepted; attacker hosts rejected; case insensitivity; malformed entries; missing/empty headers; port mismatch; operator-supplied VIEWER_ALLOWED_ORIGINS honoured).
  • 3 end-to-end tests against a real startViewerServer socket using node:http.request directly (global fetch strips Host — undici overrides it with the URL authority, so it cannot simulate the rebind payload). Each test confirms an attacker Host is rejected with 403 forbidden host before the proxy runs and that legitimate loopback hosts still serve the viewer HTML.

Verification

• npx vitest run test/viewer-security.test.ts → 13/13 pass
• npx vitest run (same exclusion set the npm test script uses) → 867/884 pass / 17 skipped / 0 fail
• The only failing file in npm run test:all is test/integration.test.ts and only because no live agentmemory server is reachable — identical failure mode against main.
• tsc --noEmit --skipLibCheck introduces zero new errors over main.

Sibling findings flagged for awareness (not patched)

• integrations/hermes/__init__.py:112 / :135 — urllib.urlopen(req) flagged by semgrep dynamic-urllib-use-detected. The _validate_url scheme allowlist (line 94-97) closes the file:// class via Python 3.7+ redirect-handler scheme restrictions; left alone deliberately.
• src/functions/replay.ts:24-34 — SENSITIVE_PATH_PATTERNS uses (s?$) boundary which mis-fires on Pascal-case (/SecretData/... doesn't match) and run-on (/secretSocialMedia.jsonl doesn't match). Low impact (local LLM data exposure on mem::compress-file), but worth a regex tightening if you want it bundled — happy to send a follow-up patch.

Detected by

Aeon (vuln-scanner + manual review). Scanners on this run: semgrep=ok (2 candidates dropped, hermes urllib above); trufflehog=ok (0 verified secrets, filesystem + full git history); osv-scanner=ok on website/package-lock.json (only postcss@8.4.31 GHSA-qx2v-qp2m-jg93 MODERATE out of scope per SECURITY.md). Main repo has no committed lockfile so dep-CVE coverage stays at version-range granularity until one ships.

Summary by CodeRabbit

• Security Improvements

  • Viewer server now validates incoming Host headers and responds 403 with body "forbidden host" for disallowed values.
  • Accepts loopback hosts (including IPv6 and explicit ports) and entries from a configurable allowlist via environment variable.

• Tests

  • Expanded test suite with unit and end-to-end tests covering host validation, case-insensitive matching, malformed origins, and various Host header scenarios.

[vercel]

Someone is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d445bc6-fcb5-4319-9420-30f458b04caf

📥 Commits

Reviewing files that changed from the base of the PR and between bd5c50f and db0821c.

📒 Files selected for processing (2)

• src/viewer/server.ts
• test/viewer-security.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/viewer/server.ts
• test/viewer-security.test.ts

---

📝 Walkthrough

Walkthrough

The viewer server now enforces DNS rebinding defense by validating incoming Host headers against an allowlist of loopback addresses and configured CORS origins. Requests with disallowed hosts receive HTTP 403 before proxying to the REST API.

Changes

DNS Rebinding Defense via Host Header Allowlist

Layer / File(s)	Summary
Host Allowlist Helpers src/viewer/server.ts	New buildAllowedHosts(origins, listenPort) and isHostAllowed(headerHost, allowed) functions parse CORS origins and environment VIEWER_ALLOWED_HOSTS, normalize to lowercase, inject loopback variants (localhost, 127.0.0.1, [::1]) for the computed listen port, and provide case-insensitive Host header validation.
Server Request Validation src/viewer/server.ts	startViewerServer lazily computes the allowed-hosts set on first request using server.address(), checks req.headers.host against the allowlist, and immediately rejects with HTTP 403 and "forbidden host" body before any proxying occurs for disallowed hosts.
Host Allowlist Unit Tests test/viewer-security.test.ts	Test suite exercises buildAllowedHosts and isHostAllowed with loopback host:port combinations, REST server port variants, rejection of attacker/rebinding hostnames, missing/empty/non-string headers, case-insensitive matching, tolerance of malformed origin entries, and honoring operator-supplied allowed origins.
E2E DNS Rebinding Tests test/viewer-security.test.ts	End-to-end tests start the viewer server on an ephemeral port, use node:http to send requests with explicit Host headers simulating rebinding attacks, and assert 403 with "forbidden host" for attacker-controlled hosts (including / landing page) and 200 or 404 for loopback hosts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 A rabbit hops through host headers with care,
Blocking rebinding attacks through the air,
Loopback addresses now guard the gate,
DNS tricksters must find a new fate,
Safe passage for localhost—hooray! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly describes the main security fix: rejecting non-loopback Host headers in the viewer server to address DNS rebinding vulnerability, which matches the core objective of the PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/viewer/server.ts (1)

117-136: 💤 Low value

Consider initializing allowedHosts on the listening event instead of lazily per request.

Functionally correct, but moving the allowlist computation into a server.on('listening', ...) handler eliminates the per-request null check and the port-fallback branch (which is unreachable in practice — requests cannot arrive before listen completes). It also makes the invariant "allowedHosts is always set when handling a request" obvious at the type level.

♻️ Proposed refactor

-  // Computed lazily on first request — `port` may be 0 here (OS-assigned),
-  // in which case we don't know the real listen port until after listen()
-  // resolves. server.address() is the authoritative source.
-  let allowedHosts: Set<string> | null = null;
+  // Populated on the 'listening' event so we can resolve an OS-assigned
+  // port (port=0) via server.address() before the first request.
+  let allowedHosts: Set<string> = new Set();

   const server = createServer(async (req, res) => {
-    if (!allowedHosts) {
-      const addr = server.address();
-      const actualPort =
-        addr && typeof addr === "object" && "port" in addr
-          ? (addr.port as number)
-          : port;
-      allowedHosts = buildAllowedHosts(ALLOWED_ORIGINS, actualPort);
-    }
     if (!isHostAllowed(req.headers.host, allowedHosts)) {
       res.writeHead(403, { "Content-Type": "text/plain" });
       res.end("forbidden host");
       return;
     }
@@
   server.listen(port, "127.0.0.1", () => {
+    const addr = server.address();
+    const actualPort =
+      addr && typeof addr === "object" && "port" in addr
+        ? (addr.port as number)
+        : port;
+    allowedHosts = buildAllowedHosts(ALLOWED_ORIGINS, actualPort);
     console.log(`[agentmemory] Viewer: http://localhost:${port}`);
   });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/viewer/server.ts` around lines 117 - 136, Move the lazy computation of
allowedHosts into a server 'listening' handler so allowedHosts is initialized
once after the real listen port is known: add server.on('listening', ...) that
calls server.address(), computes actualPort, and sets allowedHosts =
buildAllowedHosts(ALLOWED_ORIGINS, actualPort); then remove the per-request
initialization and null check inside the createServer handler and rely on
isHostAllowed(req.headers.host, allowedHosts) (ensuring allowedHosts is non-null
by the time requests are handled).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/viewer/server.ts`:
- Around line 117-136: Move the lazy computation of allowedHosts into a server
'listening' handler so allowedHosts is initialized once after the real listen
port is known: add server.on('listening', ...) that calls server.address(),
computes actualPort, and sets allowedHosts = buildAllowedHosts(ALLOWED_ORIGINS,
actualPort); then remove the per-request initialization and null check inside
the createServer handler and rely on isHostAllowed(req.headers.host,
allowedHosts) (ensuring allowedHosts is non-null by the time requests are
handled).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc2fcabb-a5ab-4629-bcc2-abf111c45e29

📥 Commits

Reviewing files that changed from the base of the PR and between 03fb42d and bd5c50f.

📒 Files selected for processing (2)

• src/viewer/server.ts
• test/viewer-security.test.ts

[aaronjmars]

Just rebased onto latest main to clear the merge conflict — branch is mergeable now. cc @rohitg00, happy to address any feedback whenever you have a moment.