feat: add --host flag and AGENTMEMORY_HOST for configurable bind address#194
feat: add --host flag and AGENTMEMORY_HOST for configurable bind address#194Edison-A-N wants to merge 2 commits intorohitg00:mainfrom
Conversation
…address Add opt-in support for binding agentmemory services to non-localhost addresses (e.g. 0.0.0.0 for LAN access). Default remains 127.0.0.1 per security advisory GHSA rohitg00#3. - CLI: --host <addr> flag sets AGENTMEMORY_HOST env - Config: new host field in AgentMemoryConfig (default 127.0.0.1) - Viewer: startViewerServer accepts host param, binds accordingly - iii-engine: CLI patches iii-config.yaml at startup when host is overridden, creating a temp config with the new bind address - CORS: viewer dynamically allows origins on configured ports when bound to 0.0.0.0 - CSP: connect-src relaxed to wildcard when externally bound - Security: warns if --host is non-localhost without AGENTMEMORY_SECRET
|
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. |
📝 WalkthroughWalkthroughAdds configurable host binding across the app: a new CLI Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI
participant Config as Config Loader
participant Engine as iii-engine
participant Viewer as Viewer Server
participant Auth as Auth/CSP
User->>CLI: run with --host <addr>
CLI->>Config: loadConfig() (reads AGENTMEMORY_HOST)
Config-->>CLI: returns config(host,...)
alt host != 127.0.0.1
CLI->>Engine: patch iii config (host, allowed_origins)
Engine->>Engine: write patched temp config
CLI->>Engine: startEngine(--config patched)
else
CLI->>Engine: startEngine(--config original)
end
CLI->>Viewer: startViewerServer(port, host=config.host)
Viewer->>Auth: buildViewerCsp(AGENTMEMORY_HOST)
alt host == 0.0.0.0
Auth-->>Viewer: connect-src *
Viewer->>Viewer: allow origins matching REST/Viewer ports
else
Auth-->>Viewer: connect-src 'self' + localhost origins
Viewer->>Viewer: allow ALLOWED_ORIGINS
end
Viewer-->>CLI: server started (logs use host display)
CLI-->>User: print endpoints
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Ensures the warning about binding without AGENTMEMORY_SECRET is shown even when --no-engine is used.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/viewer/server.ts (1)
144-147:⚠️ Potential issue | 🟡 Minor
displayHostmirrors the same too-narrow check assrc/index.ts.Identical nit: non-loopback non-
0.0.0.0binds loghttp://localhost:<port>, which is wrong for external addresses. Suggest matching whatever approach you settle on insrc/index.tsline 137 so the viewer URL reported here is reachable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/viewer/server.ts` around lines 144 - 147, The logged displayHost in the server.listen callback incorrectly maps any non-"0.0.0.0" bind to "localhost"; change the logic so only "0.0.0.0" maps to "localhost" and all other resolvedHost values are shown as-is (i.e., in the server.listen callback use resolvedHost === "0.0.0.0" ? "localhost" : resolvedHost) so the Viewer URL printed reflects the actual bind address; update the code around server.listen and the displayHost variable accordingly to match the approach used in src/index.ts line 137.
🧹 Nitpick comments (2)
src/cli.ts (2)
344-352: LGTM, small note on the"127.0.0.1"comparison.The warning condition misses
"localhost"and"::1"as safe values — a user passing--host localhostwould get a spurious "exposes your memory store to the network" warning. Consider matching against a small set of loopback strings ("127.0.0.1","localhost","::1") instead of only"127.0.0.1".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 344 - 352, The host check currently treats only "127.0.0.1" as safe and will warn incorrectly for other loopback values; update the check around hostOverride in src/cli.ts to treat a small set of loopback strings as safe (e.g., "127.0.0.1", "localhost", "::1") before emitting the p.log.warn about AGENTMEMORY_SECRET, so if hostOverride matches any of those values the warning is skipped; keep the existing secret lookup (process.env["AGENTMEMORY_SECRET"]) and p.log.warn message unchanged otherwise.
218-273: Duplicate host-override block instartEngine.Lines 219-226 and 263-270 are byte-identical. Extract a helper so the two engine-start paths stay in sync (any future change — e.g., validation, logging, fallback — otherwise has to be made twice).
Proposed refactor
+ function resolveEngineConfig(configPath: string): string { + const hostOverride = process.env["AGENTMEMORY_HOST"]; + return hostOverride && hostOverride !== "127.0.0.1" + ? patchIiiConfigHost(configPath, hostOverride) + : configPath; + }Then replace both blocks with
const effectiveConfig = resolveEngineConfig(configPath);.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 218 - 273, The startEngine code contains two identical blocks that compute hostOverride/effectiveConfig and start iii-engine (the duplicated logic using patchIiiConfigHost and spawnEngineBackground); extract that logic into a small helper (e.g., resolveEngineConfig(configPath) that applies process.env["AGENTMEMORY_HOST"] via patchIiiConfigHost and returns the effectiveConfig, or a helper startEngineWithBin(iiiBin, configPath) that computes effectiveConfig and calls spawnEngineBackground), then replace both duplicated blocks with a single call to the new helper so validation/logging/fallback changes (involving patchIiiConfigHost, spawnEngineBackground and the spinner calls) are made only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/auth.ts`:
- Around line 17-21: The CSP logic in auth.ts currently only treats the literal
host "0.0.0.0" as permissive (connect-src *), which breaks LAN/host bindings
like "192.168.1.5"; change the host-handling rule to treat any non-loopback host
as external and derive allowed origins from the configured host and ports
(instead of hardcoding "0.0.0.0"), then apply this same rule in auth.ts
(variable host and connectSrc) and update viewer/server.ts (isAllowedOrigin) and
cli.ts to compute and return the same allowed origin list so viewer fetch/WS
calls to the bound host:port are included in CSP and isAllowedOrigin checks.
Ensure the implementation checks for loopback/localhost (127.0.0.1, ::1,
"localhost") vs non-loopback and builds connect-src and origin allowlists from
the actual host + ports.
In `@src/cli.ts`:
- Around line 113-127: patchIiiConfigHost currently does brittle global text
replacements; instead parse the YAML (e.g., using the project's YAML loader or a
small `yaml` lib) after readFileSync, update only the top-level `host` and
`allowed_origins` keys (handle quoted values and multi-line arrays properly),
validate that the expected keys existed and throw or return an error if not
(don’t silently no-op), write the patched YAML to a temp file with restrictive
perms (writeFileSync(..., { mode: 0o600 })) and avoid leaking temp dirs (remove
the tmp dir on process exit or use a secure tmp file API), and keep references
to the existing symbols (`patchIiiConfigHost`, `mkdtempSync`, `writeFileSync`,
`readFileSync`, `vlog`) so you replace the regex logic with YAML
parse/mutate/serialize and explicit error handling.
In `@src/index.ts`:
- Around line 137-141: Startup logs incorrectly map any non-"0.0.0.0" host to
"localhost", misleading users; change the displayHost logic so it shows the
actual configured host (config.host) except when you intentionally want to show
"0.0.0.0" (keep the existing special-case for "0.0.0.0" if desired).
Specifically, update the const displayHost in src/index.ts (the variable used in
the two console.log calls) to use config.host directly when it's not "0.0.0.0"
(e.g., const displayHost = config.host === "0.0.0.0" ? "0.0.0.0" : config.host),
and apply the equivalent change to the displayHost/logging code in the viewer
server where the same pattern is used (the console.log/ws log there). Ensure the
console.log lines continue to use displayHost and ports unchanged.
In `@src/viewer/server.ts`:
- Around line 18-33: isAllowedOrigin currently trusts any hostname if the port
matches when AGENTMEMORY_HOST === "0.0.0.0"; tighten it by also validating the
parsed URL hostname against an allowlist: keep the existing ALLOWED_ORIGINS
check, add support for a new VIEWER_ALLOWED_ORIGINS env var (parse CSV into a
set), and when AGENTMEMORY_HOST is "0.0.0.0" only return true if url.hostname is
loopback (127.0.0.1, ::1), equals the configured AGENTMEMORY_HOST (when not
wildcard) or is contained in VIEWER_ALLOWED_ORIGINS and the port equals restPort
or viewerPort; update isAllowedOrigin to use these checks and reference
process.env.III_REST_PORT, parseInt logic, and the existing ALLOWED_ORIGINS
constant.
---
Duplicate comments:
In `@src/viewer/server.ts`:
- Around line 144-147: The logged displayHost in the server.listen callback
incorrectly maps any non-"0.0.0.0" bind to "localhost"; change the logic so only
"0.0.0.0" maps to "localhost" and all other resolvedHost values are shown as-is
(i.e., in the server.listen callback use resolvedHost === "0.0.0.0" ?
"localhost" : resolvedHost) so the Viewer URL printed reflects the actual bind
address; update the code around server.listen and the displayHost variable
accordingly to match the approach used in src/index.ts line 137.
---
Nitpick comments:
In `@src/cli.ts`:
- Around line 344-352: The host check currently treats only "127.0.0.1" as safe
and will warn incorrectly for other loopback values; update the check around
hostOverride in src/cli.ts to treat a small set of loopback strings as safe
(e.g., "127.0.0.1", "localhost", "::1") before emitting the p.log.warn about
AGENTMEMORY_SECRET, so if hostOverride matches any of those values the warning
is skipped; keep the existing secret lookup (process.env["AGENTMEMORY_SECRET"])
and p.log.warn message unchanged otherwise.
- Around line 218-273: The startEngine code contains two identical blocks that
compute hostOverride/effectiveConfig and start iii-engine (the duplicated logic
using patchIiiConfigHost and spawnEngineBackground); extract that logic into a
small helper (e.g., resolveEngineConfig(configPath) that applies
process.env["AGENTMEMORY_HOST"] via patchIiiConfigHost and returns the
effectiveConfig, or a helper startEngineWithBin(iiiBin, configPath) that
computes effectiveConfig and calls spawnEngineBackground), then replace both
duplicated blocks with a single call to the new helper so
validation/logging/fallback changes (involving patchIiiConfigHost,
spawnEngineBackground and the spinner calls) are made only once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b960b203-4dbf-4e30-b8ad-9156ef90a883
📒 Files selected for processing (6)
src/auth.tssrc/cli.tssrc/config.tssrc/index.tssrc/types.tssrc/viewer/server.ts
| const host = process.env.AGENTMEMORY_HOST; | ||
| const connectSrc = | ||
| host === "0.0.0.0" | ||
| ? "connect-src *" | ||
| : "connect-src 'self' http://localhost:* http://127.0.0.1:* ws://localhost:* ws://127.0.0.1:* wss://localhost:* wss://127.0.0.1:*"; |
There was a problem hiding this comment.
CSP relaxation is too narrow — only "0.0.0.0" gets connect-src *; binding to a LAN/interface IP breaks the viewer.
The --host <addr> flag accepts arbitrary addresses (e.g., 192.168.1.5, 10.0.0.4, or a hostname), and src/cli.ts treats anything other than 127.0.0.1 as an external bind. But here only the literal string "0.0.0.0" triggers the permissive connect-src. A user who follows the advertised flow with --host 192.168.1.5 will load the viewer from http://192.168.1.5:3113 and its fetch/WS calls to http://192.168.1.5:3111 will be blocked by CSP, because that origin isn't in the fixed allowlist.
Decide on one host-handling rule and apply it consistently across auth.ts, viewer/server.ts (isAllowedOrigin), and cli.ts (e.g., "any non-loopback host" is treated as external), or explicitly derive the allowed origins from the configured host+ports so LAN binding actually works end-to-end.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/auth.ts` around lines 17 - 21, The CSP logic in auth.ts currently only
treats the literal host "0.0.0.0" as permissive (connect-src *), which breaks
LAN/host bindings like "192.168.1.5"; change the host-handling rule to treat any
non-loopback host as external and derive allowed origins from the configured
host and ports (instead of hardcoding "0.0.0.0"), then apply this same rule in
auth.ts (variable host and connectSrc) and update viewer/server.ts
(isAllowedOrigin) and cli.ts to compute and return the same allowed origin list
so viewer fetch/WS calls to the bound host:port are included in CSP and
isAllowedOrigin checks. Ensure the implementation checks for loopback/localhost
(127.0.0.1, ::1, "localhost") vs non-loopback and builds connect-src and origin
allowlists from the actual host + ports.
| function patchIiiConfigHost(configPath: string, host: string): string { | ||
| let content = readFileSync(configPath, "utf-8"); | ||
| content = content.replace(/host:\s*127\.0\.0\.1/g, `host: ${host}`); | ||
| if (host === "0.0.0.0") { | ||
| content = content.replace( | ||
| /allowed_origins:\s*\[.*?\]/, | ||
| `allowed_origins: ["*"]`, | ||
| ); | ||
| } | ||
| const tmpDir = mkdtempSync(join(tmpdir(), "agentmemory-")); | ||
| const tmpConfig = join(tmpDir, "iii-config.yaml"); | ||
| writeFileSync(tmpConfig, content, "utf-8"); | ||
| vlog(`Patched iii-config host to ${host}: ${tmpConfig}`); | ||
| return tmpConfig; | ||
| } |
There was a problem hiding this comment.
patchIiiConfigHost can silently fail to patch the engine config.
Several fragility problems with the text-based rewrite:
- Brittle regex — silent no-ops.
/host:\s*127\.0\.0\.1/gonly matches the bare unquoted form. If the shippediii-config.yamluseshost: "127.0.0.1",host: '127.0.0.1',host: localhost, or already has a non-default value, the replacement does nothing andvlogstill reports "Patched iii-config host to X". The engine then keeps binding to its original host while the CLI/viewer/CSP all assume the override took effect — a confusing (and potentially security-relevant) inconsistency. allowed_originsregex is single-line./allowed_origins:\s*\[.*?\]/won't match a YAML array that spans multiple lines, which is a common lint-friendly formatting choice.- Unscoped key match.
host:in YAML can legitimately appear under unrelated keys (e.g., database/redis sub-configs); a global replace will rewrite all of them. - No cleanup / permissive tmpfile.
mkdtempSyncleaks one directory per invocation, and the patched copy of the engine config is written with default perms in a world-readable location (problematic if the config carries credentials).
Suggested direction: parse the YAML (a small dep like yaml is already common, or use the engine's existing loader), mutate the specific top-level host/allowed_origins keys, re-serialize, fail loudly if the expected keys aren't present, and either clean up the temp dir on exit or write with mode: 0o600.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli.ts` around lines 113 - 127, patchIiiConfigHost currently does brittle
global text replacements; instead parse the YAML (e.g., using the project's YAML
loader or a small `yaml` lib) after readFileSync, update only the top-level
`host` and `allowed_origins` keys (handle quoted values and multi-line arrays
properly), validate that the expected keys existed and throw or return an error
if not (don’t silently no-op), write the patched YAML to a temp file with
restrictive perms (writeFileSync(..., { mode: 0o600 })) and avoid leaking temp
dirs (remove the tmp dir on process exit or use a secure tmp file API), and keep
references to the existing symbols (`patchIiiConfigHost`, `mkdtempSync`,
`writeFileSync`, `readFileSync`, `vlog`) so you replace the regex logic with
YAML parse/mutate/serialize and explicit error handling.
| const displayHost = config.host === "0.0.0.0" ? "0.0.0.0" : "localhost"; | ||
| console.log( | ||
| `[agentmemory] REST API: http://localhost:${config.restPort}/agentmemory/*`, | ||
| `[agentmemory] REST API: http://${displayHost}:${config.restPort}/agentmemory/*`, | ||
| ); | ||
| console.log(`[agentmemory] Streams: ws://localhost:${config.streamsPort}`); | ||
| console.log(`[agentmemory] Streams: ws://${displayHost}:${config.streamsPort}`); |
There was a problem hiding this comment.
Log prints http://localhost:... even when bound to a non-loopback IP.
If a user runs --host 192.168.1.5 (or any non-loopback address other than 0.0.0.0), the startup banner claims http://localhost:3111 / ws://localhost:3112, which is actively misleading — localhost won't resolve to that interface from other machines.
Proposed fix
- const displayHost = config.host === "0.0.0.0" ? "0.0.0.0" : "localhost";
+ const displayHost =
+ config.host === "127.0.0.1" || config.host === "localhost"
+ ? "localhost"
+ : config.host;Same pattern is duplicated in src/viewer/server.ts (line 145) — please apply the equivalent fix there for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.ts` around lines 137 - 141, Startup logs incorrectly map any
non-"0.0.0.0" host to "localhost", misleading users; change the displayHost
logic so it shows the actual configured host (config.host) except when you
intentionally want to show "0.0.0.0" (keep the existing special-case for
"0.0.0.0" if desired). Specifically, update the const displayHost in
src/index.ts (the variable used in the two console.log calls) to use config.host
directly when it's not "0.0.0.0" (e.g., const displayHost = config.host ===
"0.0.0.0" ? "0.0.0.0" : config.host), and apply the equivalent change to the
displayHost/logging code in the viewer server where the same pattern is used
(the console.log/ws log there). Ensure the console.log lines continue to use
displayHost and ports unchanged.
| function isAllowedOrigin(origin: string): boolean { | ||
| if (ALLOWED_ORIGINS.includes(origin)) return true; | ||
| const host = process.env.AGENTMEMORY_HOST; | ||
| if (host === "0.0.0.0") { | ||
| try { | ||
| const url = new URL(origin); | ||
| const port = url.port || (url.protocol === "https:" ? "443" : "80"); | ||
| const restPort = process.env.III_REST_PORT || "3111"; | ||
| const viewerPort = String(parseInt(restPort, 10) + 2); | ||
| return port === restPort || port === viewerPort; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
CORS check only validates the port — any hostname with a matching port is accepted when bound to 0.0.0.0.
isAllowedOrigin parses the incoming origin and returns true as long as url.port === restPort || url.port === viewerPort. Hostnames are not checked at all, so an origin like http://attacker.example.com:3111 is accepted, and the proxy then reflects that origin back in Access-Control-Allow-Origin. A malicious page in the user's browser can therefore read any response from /agentmemory/* via the viewer proxy (which forwards Authorization: Bearer <AGENTMEMORY_SECRET> upstream), exfiltrating memory/observation/graph data.
Tighten the check to also restrict the hostname — e.g., accept only loopback, the configured bind host, and an explicit allowlist via VIEWER_ALLOWED_ORIGINS:
Proposed fix
function isAllowedOrigin(origin: string): boolean {
if (ALLOWED_ORIGINS.includes(origin)) return true;
const host = process.env.AGENTMEMORY_HOST;
if (host === "0.0.0.0") {
try {
const url = new URL(origin);
const port = url.port || (url.protocol === "https:" ? "443" : "80");
const restPort = process.env.III_REST_PORT || "3111";
const viewerPort = String(parseInt(restPort, 10) + 2);
- return port === restPort || port === viewerPort;
+ const portOk = port === restPort || port === viewerPort;
+ if (!portOk) return false;
+ // Require hostname to be loopback or an IP/hostname the operator
+ // explicitly trusts via VIEWER_ALLOWED_ORIGINS. Port alone is not
+ // enough — it would let any origin read the proxied API.
+ const hostname = url.hostname;
+ return (
+ hostname === "localhost" ||
+ hostname === "127.0.0.1" ||
+ hostname === "::1"
+ );
} catch {
return false;
}
}
return false;
}Operators who genuinely want LAN access should opt in by extending VIEWER_ALLOWED_ORIGINS with their explicit host.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/viewer/server.ts` around lines 18 - 33, isAllowedOrigin currently trusts
any hostname if the port matches when AGENTMEMORY_HOST === "0.0.0.0"; tighten it
by also validating the parsed URL hostname against an allowlist: keep the
existing ALLOWED_ORIGINS check, add support for a new VIEWER_ALLOWED_ORIGINS env
var (parse CSV into a set), and when AGENTMEMORY_HOST is "0.0.0.0" only return
true if url.hostname is loopback (127.0.0.1, ::1), equals the configured
AGENTMEMORY_HOST (when not wildcard) or is contained in VIEWER_ALLOWED_ORIGINS
and the port equals restPort or viewerPort; update isAllowedOrigin to use these
checks and reference process.env.III_REST_PORT, parseInt logic, and the existing
ALLOWED_ORIGINS constant.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli.ts (1)
218-273: DRY up the host-override resolution and make the "is override" check semantic.The 4-line
hostOverride/effectiveConfigblock is duplicated verbatim in the primaryiiiBin && configPathbranch (218-229) and the fallback branch afterfallbackIiiPaths()(262-273). Besides the duplication,hostOverride !== "127.0.0.1"treatslocalhost,::1, or0.0.0.0as "override" but silently no-ops the patch when the user passeslocalhost(the regex inpatchIiiConfigHostonly replaces127.0.0.1). Meanwhilesrc/index.tsonly shows the external-bind hint whenconfig.host === "0.0.0.0", so the three layers disagree on what counts as "non-local".Extracting a small helper both removes the duplication and gives you one place to normalize the semantics.
♻️ Suggested extraction
+function resolveEngineConfig(configPath: string): string { + const hostOverride = process.env["AGENTMEMORY_HOST"]; + if (!hostOverride || hostOverride === "127.0.0.1" || hostOverride === "localhost") { + return configPath; + } + return patchIiiConfigHost(configPath, hostOverride); +} @@ if (iiiBin && configPath) { - const hostOverride = process.env["AGENTMEMORY_HOST"]; - const effectiveConfig = - hostOverride && hostOverride !== "127.0.0.1" - ? patchIiiConfigHost(configPath, hostOverride) - : configPath; + const effectiveConfig = resolveEngineConfig(configPath); const s = p.spinner(); s.start(`Starting iii-engine: ${iiiBin}`); spawnEngineBackground(iiiBin, ["--config", effectiveConfig], "iii-engine"); @@ if (iiiBin && configPath) { - const hostOverride = process.env["AGENTMEMORY_HOST"]; - const effectiveConfig = - hostOverride && hostOverride !== "127.0.0.1" - ? patchIiiConfigHost(configPath, hostOverride) - : configPath; + const effectiveConfig = resolveEngineConfig(configPath); const s = p.spinner(); s.start(`Starting iii-engine: ${iiiBin}`); spawnEngineBackground(iiiBin, ["--config", effectiveConfig], "iii-engine");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 218 - 273, Create a small helper (e.g., resolveEffectiveIiiConfig or getIiiConfigForHost) to centralize the duplicated host-override logic: read process.env["AGENTMEMORY_HOST"], determine if it is a true non-local override by normalizing values (treat "127.0.0.1", "localhost", "::1" as local and treat "0.0.0.0" as non-local) and return either configPath or patchIiiConfigHost(configPath, host) accordingly; replace the duplicated 4-line blocks in both the initial iiiBin && configPath branch and the one after fallbackIiiPaths() with a call to this helper so spawnEngineBackground(...) uses the single resolved effectiveConfig.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli.ts`:
- Around line 68-72: The host parsing currently takes args[hostIdx + 1] without
validation and may accept another flag as the host; update the CLI parsing
around hostIdx/args to verify the next token exists and does not start with "-"
before setting process.env["AGENTMEMORY_HOST"]; if the token is missing or
begins with "-", emit a clear error (or usage message) and abort or skip setting
AGENTMEMORY_HOST so the engine config isn't patched with an invalid value;
reference the hostIdx, args, and AGENTMEMORY_HOST symbols when locating and
changing the logic.
---
Nitpick comments:
In `@src/cli.ts`:
- Around line 218-273: Create a small helper (e.g., resolveEffectiveIiiConfig or
getIiiConfigForHost) to centralize the duplicated host-override logic: read
process.env["AGENTMEMORY_HOST"], determine if it is a true non-local override by
normalizing values (treat "127.0.0.1", "localhost", "::1" as local and treat
"0.0.0.0" as non-local) and return either configPath or
patchIiiConfigHost(configPath, host) accordingly; replace the duplicated 4-line
blocks in both the initial iiiBin && configPath branch and the one after
fallbackIiiPaths() with a call to this helper so spawnEngineBackground(...) uses
the single resolved effectiveConfig.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| const hostIdx = args.indexOf("--host"); | ||
| if (hostIdx !== -1 && args[hostIdx + 1]) { | ||
| process.env["AGENTMEMORY_HOST"] = args[hostIdx + 1]; | ||
| } | ||
|
|
There was a problem hiding this comment.
Minor: --host consumes the next token without validation.
If the user runs agentmemory --host --verbose (or forgets the value before another flag), args[hostIdx + 1] will be "--verbose" and we'll happily set AGENTMEMORY_HOST=--verbose, patch the engine config with that value, and fail in confusing ways later. Consider rejecting values that start with - (mirrors how --port/--tools are also vulnerable, but --host is the more load-bearing one because of the CSP/CORS relaxations it unlocks).
🛡️ Proposed guard
const hostIdx = args.indexOf("--host");
-if (hostIdx !== -1 && args[hostIdx + 1]) {
- process.env["AGENTMEMORY_HOST"] = args[hostIdx + 1];
+const hostVal = hostIdx !== -1 ? args[hostIdx + 1] : undefined;
+if (hostIdx !== -1 && (!hostVal || hostVal.startsWith("-"))) {
+ p.log.error("--host requires an address argument (e.g. --host 0.0.0.0)");
+ process.exit(1);
+}
+if (hostVal) {
+ process.env["AGENTMEMORY_HOST"] = hostVal;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli.ts` around lines 68 - 72, The host parsing currently takes
args[hostIdx + 1] without validation and may accept another flag as the host;
update the CLI parsing around hostIdx/args to verify the next token exists and
does not start with "-" before setting process.env["AGENTMEMORY_HOST"]; if the
token is missing or begins with "-", emit a clear error (or usage message) and
abort or skip setting AGENTMEMORY_HOST so the engine config isn't patched with
an invalid value; reference the hostIdx, args, and AGENTMEMORY_HOST symbols when
locating and changing the logic.
|
Hi, The viewer server now supports binding to a configurable host, but its REST proxy still always forwards to 127.0.0.1. If iii-engine is configured (via --host) to bind only to a non-loopback interface, the viewer proxy will fail to reach the REST API. Severity: action required | Category: correctness How to fix: Proxy to configured host Agent prompt to fix - you can give this to your LLM of choice:
We noticed a couple of other issues in this PR as well - happy to share if helpful. Qodo code review - free for open-source. |
2 participants
Summary
Adds opt-in support for binding agentmemory services to non-localhost addresses (e.g.
0.0.0.0for LAN access). The default remains127.0.0.1per security advisory GHSA #3.Motivation
Users deploying agentmemory on remote dev servers or shared machines need to access the Viewer (port 3113), REST API (3111), and WebSocket streams (3112) from other hosts on the network. Currently this is impossible without patching source or running a reverse proxy, since all three services hardcode
127.0.0.1.Changes (6 files, ~85 lines added)
src/cli.ts: New--host <addr>flag, security warning when binding withoutAGENTMEMORY_SECRET, runtime patching ofiii-config.yamlfor iii-engine host overridesrc/config.ts: ReadAGENTMEMORY_HOSTenv var into configsrc/types.ts: Addhostfield toAgentMemoryConfiginterfacesrc/index.ts: Passhostto viewer server, display actual bind address in logssrc/viewer/server.ts: Accepthostparam instartViewerServer(), dynamic CORS origin checking when bound to0.0.0.0src/auth.ts: Relax CSPconnect-srcto*when externally boundUsage
Security Considerations
127.0.0.1)--hostflag orAGENTMEMORY_HOSTenvAGENTMEMORY_SECRETAGENTMEMORY_HOST=0.0.0.0Summary by CodeRabbit
New Features
--hostCLI option to set the server bind addressImprovements