Support internal-network-only deployment

[cass-clearly]

What changed

Changed direct server default bind host to localhost, changed Docker Compose publishing to localhost, and documented internal-network-only deployment with firewall verification.

Why

Closes #278. Block Security requires VPN/corporate-only access with no public internet exposure.

New/Changed Endpoints

No endpoints changed.

How to verify

• npm run check
• Start Remarq and verify it listens on loopback/private ingress only.

Manual testing checklist

• Server starts without errors (npm run start)
• Existing tests pass (npm test)
• Tested in browser (annotations, sidebar, highlights work)
• Tested API changes with curl (include example commands above)
• No console errors in browser DevTools

[cass-clearly]

The Minimalist — Round 1 Review

Verdict: APPROVE

Tests passed with exact command: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_minimalist_r1 npm test

I read gh pr diff 287, git diff origin/main...HEAD, and the changed files.

No totally different path recommended. For this requirement, the smallest useful path is exactly: change the unsafe default, constrain the compose publish address, and document the internal-only deployment posture. This PR does not add a new service layer, dependency, config framework, or speculative abstraction.

Process note: skipping plan review was wrong. It did not produce an overbuilt implementation here, so I am not blocking on it.

Non-blocking simplicity issue: the new HOST environment variable is only mentioned in the network isolation doc. If this causes operator confusion later, add it to the existing environment variable table instead of creating more deployment prose.

[cass-clearly]

The Architect — Round 1 Review

Verdict: APPROVE

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_architect_r1 npm test

I read gh pr diff 287, git diff origin/main...HEAD, issue #278, and the changed files.

No totally different path recommended. The implementation matches the requirement at the right level: safer direct default, loopback-only Docker publish, and explicit private ingress/firewall guidance. Anything more here would be pretending application code can enforce corporate network topology. It cannot.

Process violation: plan review was skipped. That was wrong and should not become precedent. It is not a blocker on this PR because the resulting change is small, coherent, and reversible.

Non-blocking problems:

• server/index.js still logs http://localhost:${port} even when HOST is set to a different value. That is misleading operator output, not a shipping blocker for this network-isolation PR.
• HOST is only documented in the new network isolation doc, not in the broader production/config docs. If operators trip over Docker/direct deployment behavior, add it there in a follow-up.
• There is no direct test for the default host selection. Given the blast radius and the existing full suite passing, I am not holding this PR for that.

Ship this. Do not expand it into a firewall/proxy implementation PR.

[cass-clearly]

The Craftsperson — Round 1 Review

Verdict: REQUEST_CHANGES

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_craftsperson_r1 npm test

I do not recommend a totally different path. Binding direct startup to loopback by default and publishing Docker Compose only on loopback is the right shape. The implementation is not done.

Required fixes:

1. Add behavior tests for the listen host resolution. This PR changes server/index.js from default-public to default-loopback, but server/test.mjs always calls start({ port: 0, host: "127.0.0.1" }). The suite would still pass if the default stayed 0.0.0.0 or if process.env.HOST were ignored. That is a Principle 5 failure: the test suite does not make the network-isolation behavior safe to change.

   • Cover default host with no option/env => 127.0.0.1.
   • Cover HOST env override.
   • Cover explicit options.host overriding env.
   • Preserve the existing port: 0 behavior.
   • Prefer extracting a small pure listen-config resolver and testing that directly instead of adding another DB-backed integration test.

2. Fix the startup log while touching this code. start({ port: 0, host: "127.0.0.1" }) currently logs Remarq server listening on http://localhost:0; with HOST=0.0.0.0 it would still say localhost. That message is now part of deployment/debugging behavior and it lies about the bound address/port. Use server.address() after listen or stop printing a concrete URL.

This is a missed red-green-refactor cycle: the production line changed, but no failing test describes the new contract, and the nearby misleading log was left in place.

[cass-clearly]

The Marketing Guru — Round 1 Review

Verdict: REQUEST_CHANGES

Tests passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_marketing-guru_r1 npm test.

I fetched and read gh pr diff 287, git diff origin/main...HEAD, and the changed/relevant docs and source files.

Plan review was skipped. From the implementation, I do not recommend a totally different path; loopback-by-default plus internal ingress guidance is the right direction. The shipping problem is documentation completeness and reader trust.

Required fixes:

1. Add a CHANGELOG.md entry under [Unreleased] for this change. This PR changes a deployment default (0.0.0.0 -> 127.0.0.1) and the supported enterprise security posture. Users must see that in release notes without reading the diff. Put it under Security or Changed, and call out the new HOST override.

2. Update the public/agent-facing setup docs that still describe the old or incomplete behavior. At minimum:

   • docs/best-practices.md environment variables table must include HOST, default 127.0.0.1, and when to set 0.0.0.0.
   • docs/llms-full.txt still says only “Backend runs on port 3333” and its environment table omits HOST. Agents reading this will give users outdated deployment advice.

3. Remove Closes #278 from docs/network-isolation.md. Public docs are for operators, not GitHub bookkeeping. Keep issue closure in the PR body/changelog, not the product documentation.

4. Tighten docs/network-isolation.md verification so the right check is obvious. ss -ltnp | grep 3333 is not enough; say what passing output looks like (127.0.0.1:3333) and what fails the check (0.0.0.0:3333 or :::3333). Also replace shorthand like LB/corp with clear terms (load balancer, corporate network) on first read.

Principles: 9 and 15. The right deployment posture must be easy to verify, and slow/smooth here means every public doc and release note tells the same story.

[cass-clearly]

The Steward — Round 1 Review

Verdict: REQUEST_CHANGES

Test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_steward_r1 npm test.

I read gh pr diff 287, git diff origin/main...HEAD, and the changed files.

Skipping plan review was a process failure. I do not recommend a totally different path: making localhost the default and constraining Compose host publishing is the right direction. The implementation is not production-ready yet.

Required fixes:

1. Add regression tests for the bind contract. The security behavior lives in server/index.js:650-652, but the suite only starts the API with an explicit host: "127.0.0.1". Add tests that start without options.host and assert the server binds to 127.0.0.1, and that process.env.HOST / options.host override the default. This is the core requirement; without a test, a future change can silently reopen the public listener.

2. Fix deployment docs so operators do not follow contradictory production guidance. docs/best-practices.md:38-91 still presents generic production reverse-proxy examples with remarq.example.com and no VPN/corporate CIDR restriction, and its environment table omits HOST. README.md:112-123 also shows direct startup without explaining that it now binds localhost unless HOST is set. Document the new default, the override, and the internal-only/VPN requirement wherever production startup is documented.

3. Fix the startup log. server/index.js:679 always logs http://localhost:${port} even when Compose intentionally sets HOST=0.0.0.0 or an operator binds a private interface. For a network-isolation change, logs cannot hide the actual bind address. Log the actual address/port from server.address() or the resolved host.

Principles: API stability is not just endpoints; the bind address is an operational contract. The right thing is not easy if HOST is undocumented in the primary deployment docs. Slow is smooth: lock the security default with tests and make the operator-facing signals accurate before shipping.

[cass-clearly]

The Minimalist — Round 2 Review

Verdict: APPROVE

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_minimalist_r2 npm test

I fetched and read the current PR diff and changed files.

No totally different path is warranted. The PR still takes the smallest useful path for #278: safer loopback default, localhost-only Compose publication, and concise operator guidance for internal-only deployment. Round 2's resolveListenOptions extraction is a small test seam, not an architecture layer; it avoids a heavier integration test and locks the bind contract directly.

No Minimalist blockers. Do not expand this into firewall/proxy enforcement in application code.

[cass-clearly]

The Craftsperson — Round 2 Review

Verdict: APPROVE

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_craftsperson_r2 npm test

I fetched and read the current PR diff and changed files.

Round 1's code-quality blockers are fixed. The listen-host rule is now extracted into resolveListenOptions, which gives the suite a small pure seam for the behavior that matters: default loopback, HOST env override, explicit host override, and preservation of port: 0. That is the right refactor shape; adding a DB-backed integration test would have coupled this security contract to unrelated server startup cost.

The startup log now reports the actual bound address and port from server.address(), so the operator-facing output no longer lies when the OS assigns a port or when HOST changes.

No totally different path is warranted. The implementation is small, named, and covered at the behavior boundary. From the Craftsperson lens, this has completed the red-green-refactor step well enough to ship.

[cass-clearly]

The Steward — Round 2 Review

Verdict: REQUEST_CHANGES

Test command passed exactly: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_steward_r2 npm test.

I fetched and read the current PR diff and changed files.

No totally different path is warranted. The implementation path is still right: loopback default, loopback-only Compose host publishing, a HOST override for controlled deployments, and operator documentation.

Round 1 production-readiness fixes are mostly addressed: the bind contract now has a small resolver test seam, Compose is constrained at the host port, and startup logs now print the actual bound address/port.

Required fixes:

1. docs/best-practices.md still gives production reverse-proxy guidance as a public-domain deployment. Lines 60-98 show remarq.example.com, say to point the script tag at the "public domain", and do not mention VPN/corporate ingress, CIDR allowlisting, or the network-isolation doc. That contradicts this PR's supported enterprise posture. Update this section so the easy path is internal-only: internal DNS/domain wording, explicit VPN/corporate CIDR restriction, and/or a direct link to docs/network-isolation.md before the examples.

2. docs/llms-full.txt is still stale agent-facing setup documentation. It says only "Backend runs on port 3333" and its environment table omits HOST. Agents reading this file will give operators the old deployment model and miss the new bind contract. Update it with the localhost default, HOST override, and internal-only deployment guidance.

Principles: 8, 9, and 15. The network bind address is an operational contract. If the primary production guide and LLM-facing docs still imply public deployment or omit HOST, the right deployment posture is not the easy one to follow.

[cass-clearly]

The Marketing Guru — Round 2 Review

Verdict: REQUEST_CHANGES

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_marketing-guru_r2 npm test

I fetched and read the current PR diff and changed files.

No totally different path is warranted. Loopback-by-default, localhost-only Compose publishing, and internal-ingress documentation are the right shape for #278. Round 2 fixed the changelog, removed GitHub bookkeeping from public docs, tightened verification, and documented HOST in docs/best-practices.md.

One material docs blocker remains:

1. docs/llms-full.txt is still outdated. It still says “Backend runs on port 3333” and its environment table still omits HOST. This is explicitly agent-facing documentation; agents reading it will give users incomplete network-isolation advice. Update it to match the README/best-practices story: default bind host is 127.0.0.1, Docker Compose publishes localhost only, and HOST should only be set for private network/trusted reverse-proxy deployments.

Also fix the production wording in docs/best-practices.md while you are there: the reverse-proxy section still uses remarq.example.com / “public domain” without saying the proxy must be internal/VPN/corporate-only. For a network-isolation PR, public docs cannot invite the opposite deployment posture.

Principles: 9 and 15. The right deployment posture must be the easy one, and every human/agent-facing setup doc needs to tell the same story before this ships.

[cass-clearly]

The Architect — Round 2 Review

Verdict: REQUEST_CHANGES

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_architect_r2 npm test.

I fetched and read the current PR diff and changed files.

No totally different path is warranted. The runtime shape is right: default direct startup to loopback, Docker Compose publishes on loopback while the container binds internally, the bind contract is now covered by a small resolver test, and the startup log reports the actual address/port. Application code cannot enforce corporate firewall topology; trying to build that here would be the wrong path.

Material blocker: the deployment story is still not coherent enough to close #278. This PR is primarily about supported deployment posture, so contradictory primary docs are not a polish issue.

Required fixes:

1. docs/best-practices.md still presents the production reverse-proxy path as a public remarq.example.com deployment and says to point the script tag at the public domain, without the VPN/corporate ingress restriction or a link to docs/network-isolation.md. Add the internal-network-only constraint there, use internal/VPN wording, and make the safe path the obvious path.

2. docs/llms-full.txt is still stale: it says only “Backend runs on port 3333,” omits HOST from the environment table, and does not mention the internal-network-only posture. That is an agent-facing setup surface; leaving it stale means agents will keep giving old deployment advice. Update or regenerate it so it tells the same story as README and docs/network-isolation.md.

Do not expand this into firewall/proxy implementation. Fix the doc coherence and ship.

[cass-clearly]

The Minimalist — Round 3 Review

Verdict: APPROVE

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_minimalist_r3 npm test

I fetched and read the current PR diff and changed files.

No totally different path is warranted. The PR remains the smallest useful implementation for #278: default direct startup to loopback, Docker Compose published only on localhost, and a short internal-network-only deployment doc with verification steps. It does not try to fake firewall enforcement in application code.

The resolveListenOptions extraction is justified as a tiny pure test seam; deleting it would either remove coverage of the bind contract or force a heavier server-start integration test. The docs edits are broader than code changes, but they replace contradictory deployment guidance rather than adding speculative options.

No Minimalist blockers. Stop here; do not expand this into proxy/firewall implementation or more configuration surface.

[cass-clearly]

The Craftsperson — Round 3 Review

Verdict: APPROVE

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_craftsperson_r3 npm test.

I fetched and read the current PR diff, changed files, prior council comments, and issue #278.

No totally different path is warranted. The implementation still has the right craft shape: the bind-address rule is isolated in a small pure resolver, covered with behavior-focused tests for default loopback, env override, explicit override, and port: 0, and start() remains straightforward. The startup log now uses server.address(), so the observable behavior matches the resolved bind address instead of carrying a stale hard-coded localhost/port message.

Round 3 did not introduce code churn; it tightened the deployment docs. From the Craftsperson lens, the red-green-refactor gap from Round 1 is closed and there are no material code-quality or test-quality blockers.

[cass-clearly]

The Steward — Round 3 Review

Verdict: APPROVE

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_steward_r3 npm test.

I fetched and read the current PR diff and changed files.

No totally different path is warranted. The application can make the safe bind behavior the default and document the required network perimeter; it should not try to implement corporate firewall/VPN policy itself.

Round 2 blockers are addressed:

• The bind contract is covered by resolveListenOptions tests: default loopback, HOST env override, explicit host override, and port: 0 preservation.
• Startup logs now report the actual bound address and port from server.address(), so operator output matches runtime behavior.
• docs/best-practices.md now frames deployment as internal-only, links the network-isolation doc, documents HOST, and replaces the public-domain examples with internal-domain usage.
• docs/llms-full.txt now states the localhost bind/default Docker publish behavior and includes HOST in the environment table, so agent-facing setup docs no longer omit the operational contract.

From the Steward lens: this is a controlled, additive operational contract with an intentional security default change called out in release/docs. The right deployment path is now visible enough for operators, and the blast radius is constrained to configuration (HOST) rather than API behavior.

[cass-clearly]

The Marketing Guru — Round 3 Review

Verdict: REQUEST_CHANGES

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_marketing-guru_r3 npm test.

I fetched and read the current PR diff and changed files.

No totally different path is warranted. The product shape is right: default loopback binding, localhost-only Compose publishing, a controlled HOST override, and an internal-ingress deployment guide.

One material docs blocker remains:

1. docs/llms-full.txt still teaches public-looking deployment examples. It now mentions the localhost bind/default HOST, but it still says Docker Compose is “recommended for production,” tells agents to replace SERVER with https://remarq.example.com, and uses REMARQ_URL = "https://remarq.example.com" in the agent examples. That contradicts the internal-only story now fixed in docs/best-practices.md. This file is explicitly agent-facing; agents will copy those examples into operator guidance.

Fix: update docs/llms-full.txt to use the same internal-network language and examples as the public docs: “internal deployment” instead of generic production, https://remarq.internal.example.com for deployed Remarq examples, and an explicit note that deployed Remarq URLs must be VPN/corporate/internal-only, not public internet endpoints.

Principles: 9 and 15. The right deployment posture is not easy if the agent-facing source of truth still shows public-style URLs for production use.

[cass-clearly]

The Architect — Round 3 Review

Verdict: APPROVE

Exact test command passed: DATABASE_URL=postgresql://remarq:remarq@localhost:5433/remarq_pr287_architect_r3 npm test.

I fetched and read the current PR diff, changed files, prior council comments, and issue #278. CI check rollup is green on the current head.

No totally different path is warranted. The coherent solution remains the small one: default direct startup to loopback, publish the Compose service only on host loopback while binding inside the container, document the internal-network-only architecture and firewall/ingress verification, and stop there. Application code cannot enforce a company VPN or firewall boundary; trying to add that here would create false confidence and unnecessary architecture.

Round 2's material blockers are resolved. The bind contract now has a focused pure test seam, startup logs report the actual bound address/port, HOST is documented in the primary deployment surfaces, and the production guide now points operators toward internal/VPN/corporate ingress instead of public exposure. The remaining docs are not perfect, but they are coherent enough for the acceptance criteria and not worth another review cycle.

Ship it.