Skip to content

Commit 6df98f3

Browse files
committed
merge: land security hardening batch 3 (login backoff behind a proxy) on beta
2 parents 5f40146 + 79df51b commit 6df98f3

15 files changed

Lines changed: 313 additions & 79 deletions

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ product overview.
1818
1919
## Unreleased
2020

21+
- Security hardening (audit batch 3): the login and approval-link backoff key each client on the
22+
header of the proxy the operator declares (new Settings → Connection → *Behind a reverse proxy*,
23+
`trustProxy` / `CG_TRUST_PROXY`: none, `cloudflare`, `xff`) instead of trusting any forwarded
24+
header that arrives on a loopback socket, and a global cap (50 failures per 10 minutes) bounds
25+
password guessing from fresh buckets. Deployments behind a proxy should set the new option;
26+
without it remote logins share one backoff bucket.
2127
- Security hardening (audit batch 1): the Slack Settings mode control no longer lets a non-admin
2228
channel manager switch a channel into Admin mode (managers keep their other controls in an
2329
Admin-mode channel and may step it down); case-variant URLs such as `/API/settings` are refused

‎FEATURES.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,21 @@
11
# ChannelGate — Features
22

3+
## Login backoff behind a reverse proxy (security audit batch 3)
4+
5+
- The admin login backoff and the approval-link token backoff key each client on the header of
6+
the proxy the operator DECLARED, never on a header a loopback socket happened to carry. Settings
7+
→ Connection → *Behind a reverse proxy* (`trustProxy` in settings.json, `CG_TRUST_PROXY` in
8+
`.env`; the setting wins) takes `""` (none: the connecting address is the client),
9+
`cloudflare` (`CF-Connecting-IP`, which cloudflared overwrites on every request) or `xff` (the
10+
LAST `X-Forwarded-For` hop, the one the trusted proxy appended, never the first, which the client
11+
typed). The legacy `CG_TRUST_PROXY=1` means `xff`. An unknown value trusts nothing; the admin API
12+
refuses it with 400.
13+
- A global failure cap bounds guessing independently of the key: more than 50 failed attempts in
14+
ten minutes across all clients puts every login on the maximum delay until the window drains, so
15+
a caller who can mint fresh buckets (a botnet, or a forged header behind a misdeclared proxy) is
16+
still bounded, and so is the scrypt work an unauthenticated caller can cause.
17+
- Documented in `docs/OPERATIONS.md` → Reverse proxies and tunnels and in INSTALL.md: without the
18+
setting, all remote logins behind a proxy share one backoff bucket.
319
## Web and Slack surface hardening (security audit batch 1)
420

521
- **Admin mode is admin-only on every surface.** The Slack Settings mode control refuses to switch

‎INSTALL.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,10 @@ For a shared workflow, start with one dedicated channel and one representative t
230230
history in the admin UI when reviewing the pilot.
231231

232232
Slack Socket Mode needs no public inbound URL. If operators need the admin UI or approval links
233-
from outside the host, put a tunnel or reverse proxy in front of the loopback service and set
234-
**Settings → Connection → Public URL** to its HTTPS origin. See
233+
from outside the host, put a tunnel or reverse proxy in front of the loopback service, set
234+
**Settings → Connection → Public URL** to its HTTPS origin, and set **Behind a reverse proxy** to
235+
the proxy you run (`cloudflare`, or `xff` for nginx/caddy) so the login backoff keys on each real
236+
client — without it, all remote logins share one backoff bucket. See
235237
[reverse proxies and tunnels](./docs/OPERATIONS.md#reverse-proxies-and-tunnels).
236238

237239
## Backup & restore config (encrypted)

‎TEST-PLAN.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,27 @@
11
# ChannelGate — Test Plan
22

3+
## Login backoff behind a reverse proxy (2026-10-25)
4+
5+
- [x] Automated: `node scripts/run-tests.mjs test/web-security.test.js test/settings-save-version.test.js
6+
test/admin-auth-bind.test.js test/approval-links.test.js` — `clientKey` ignores both forwarded
7+
headers with no proxy declared (loopback and non-loopback sockets alike); `cloudflare` reads
8+
`CF-Connecting-IP` only; `xff` reads the LAST `X-Forwarded-For` hop; the stored setting wins over
9+
`CG_TRUST_PROXY`; legacy `1`/`true`/`yes` mean `xff`; an unknown value trusts nothing; the global
10+
cap holds fresh keys at the maximum delay after N failures and drains with the window, is
11+
switchable off, and defaults to 50 per 10 minutes; the admin API persists `""`/`cloudflare`/`xff`,
12+
maps `1` to `xff`, refuses unknown values with 400 and writes nothing; the Settings page paints
13+
and saves the select.
14+
- [x] `npm run check:static`, `npm run secret-scan`.
15+
- [ ] Live, engine-independent (browser + curl): on the deployment (Cloudflare tunnel in front) set
16+
Settings → Connection → Behind a reverse proxy to **Cloudflare tunnel** and save. From a browser
17+
through the public hostname enter three wrong admin passwords; the fourth attempt is delayed
18+
(the page reports the wait) while a colleague on a different network can still sign in at once.
19+
Then, against the LAN/loopback bind only (Cloudflare overwrites the header on the public path),
20+
`for i in 1 2 3 4 5 6; do curl -s -o /dev/null -w '%{http_code}\n' -X POST http://127.0.0.1:4747/api/login
21+
-H 'content-type: application/json' -H "CF-Connecting-IP: 10.0.0.$i" -d '{"password":"wrong"}'; done`
22+
with the setting at **None**: every request shares the loopback bucket, so the 4th onwards is
23+
429/delayed regardless of the spoofed header. PASS when both hold and the setting round-trips
24+
after a daemon restart.
325
## Web and Slack surface hardening, batch 1 (2026-10-25)
426

527
- [x] Automated: `node scripts/run-tests.mjs test/web-hardening.test.js test/access-settings.test.js

‎docs/OPERATIONS.md‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -96,26 +96,29 @@ allowed past the DNS-rebinding guard (extra names go in `CG_ALLOWED_HOSTS`).
9696
**Client addresses behind that hop.** Every request then arrives on a loopback socket, so
9797
`req.ip` — and Express's idea of the client — is `127.0.0.1` for every caller on earth. Two things
9898
count failures per address: the admin login backoff and the approval-link token backoff. Keyed on
99-
the socket they would share ONE bucket, and a handful of bad tokens or passwords from anywhere
100-
would lock out every legitimate admin and every valid approval link. So both read the real client
101-
through one helper (`clientKey` in `src/web/security.js`): `CF-Connecting-IP` first, then the first
102-
hop of `X-Forwarded-For`, and **only when the socket itself is loopback** — from a non-loopback
103-
socket those headers are whatever the client typed and are ignored. A value that is not a valid IP
104-
address is ignored too.
99+
the socket they share ONE bucket, and a handful of bad tokens or passwords from anywhere delays
100+
every legitimate admin and every valid approval link. So **declare the proxy** in *Settings →
101+
Connection → Behind a reverse proxy* (`trustProxy` in settings.json, or `CG_TRUST_PROXY` in
102+
`.env`; the setting wins): `cloudflare` makes both limiters key on `CF-Connecting-IP`, which
103+
cloudflared overwrites on every request; `xff` keys on the **last** hop of `X-Forwarded-For`, the
104+
one your nginx/caddy appended (never the first, which the client typed). Without it, all remote
105+
logins share one backoff bucket. A loopback socket on its own is deliberately NOT taken as proof
106+
of a proxy — any process on the host or any tunnel opens one — so an undeclared header is ignored,
107+
and a value that is not a valid IP address is ignored too. Independently of the key, more than 50
108+
failed attempts in ten minutes across all clients puts every login on the maximum delay until the
109+
window drains, which bounds guessing even where a header is misdeclared.
105110

106111
Notes for operators:
107112

108113
- Express's app-wide `trust proxy` is deliberately NOT enabled: it would also re-point
109114
`req.secure` / `req.protocol` / `req.hostname` at client-supplied headers (the Host/Origin guard
110115
and the session cookie's `Secure` flag read those headers themselves, deliberately), and it knows
111-
nothing about `CF-Connecting-IP`.
112-
- If your proxy runs on ANOTHER host, the socket is not loopback and the headers are ignored by
113-
default. Set `CG_TRUST_PROXY=1` in `.env` to honour `X-Forwarded-For` there — only do this when a
114-
proxy you control rewrites that header, or clients can pick their own limiter bucket.
115-
- Make the proxy set `X-Forwarded-For` rather than append to a client-supplied one. Where it
116-
appends (Cloudflare does), `CF-Connecting-IP` is the unforgeable value and is preferred here; a
117-
caller who spoofs the first XFF hop can only shed its own backoff, never push someone else into
118-
one.
116+
nothing about `CF-Connecting-IP`. The `Secure` cookie decision still reads `x-forwarded-proto`
117+
on its own; that only ever makes the cookie stricter.
118+
- Only declare a proxy you control and that sets the header on EVERY request. If clients can reach
119+
the daemon's port directly (a LAN bind) while `xff` is declared, a caller could append its own
120+
hop; keep the bind on loopback or firewall the port, as the rest of this guide assumes.
121+
- The legacy `CG_TRUST_PROXY=1` still works and means `xff`.
119122

120123
## The ChannelGate rename migration
121124

‎public/app.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3891,6 +3891,7 @@ function readSettingsForm() {
38913891
composioMcpUrl: document.getElementById("set-composio").value,
38923892
toolboxMcpUrl: document.getElementById("set-toolbox").value,
38933893
publicUrl: document.getElementById("set-public-url").value,
3894+
trustProxy: document.getElementById("set-trust-proxy").value,
38943895
approvalLinks: document.getElementById("set-approval-links").value,
38953896
aiTestingUsers: aiTestingPicker?.getValues() || [],
38963897
...(document.getElementById("set-gchat-key").value.trim() ? { googleChatServiceAccountJson: document.getElementById("set-gchat-key").value } : {}),
@@ -4020,6 +4021,7 @@ function paintSettings(s) {
40204021
document.getElementById("set-composio").value = s.composioMcpUrl || "";
40214022
document.getElementById("set-toolbox").value = s.toolboxMcpUrl || "";
40224023
document.getElementById("set-public-url").value = s.publicUrl || "";
4024+
document.getElementById("set-trust-proxy").value = s.trustProxy || "";
40234025
document.getElementById("set-approval-links").value = s.approvalLinks || "auto";
40244026
aiTestingPicker = mountUserPicker(document.getElementById("set-ai-testing-users"), {
40254027
users: USERS, selected: s.aiTestingUsers || [], onChange: markSettingsDirty,

‎public/index.html‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,15 @@ <h4>Public URL</h4>
539539
<span>Public URL (the daemon's externally-reachable base URL)</span>
540540
<input id="set-public-url" type="text" placeholder="https://gateway.makeitfuture.com" />
541541
</label>
542+
<label class="field">
543+
<span>Behind a reverse proxy — which header names the real client (login and approval-link backoff)</span>
544+
<select id="set-trust-proxy">
545+
<option value="">None — the connecting address is the client</option>
546+
<option value="cloudflare">Cloudflare tunnel (CF-Connecting-IP)</option>
547+
<option value="xff">nginx / caddy / other (last X-Forwarded-For hop)</option>
548+
</select>
549+
</label>
550+
<p class="desc">Without this, every remote login shares one backoff bucket behind the proxy; with it, each real client gets its own. Only declare a proxy you control: it must set the header on every request.</p>
542551
</div>
543552
<div class="setcard">
544553
<h4>Approval links</h4>

‎src/config/settings.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,25 @@ export function getApprovalLinks() {
262262
return APPROVAL_LINK_MODES.includes(v) ? v : "auto";
263263
}
264264

265+
// Which forwarded-client header the login and approval-link backoff may believe. "" trusts
266+
// nothing (the socket address is the client); "cloudflare" reads `CF-Connecting-IP`, which
267+
// cloudflared overwrites on every request; "xff" reads the LAST hop of `X-Forwarded-For` — the one
268+
// the trusted proxy appended — never the first, which the client typed. The legacy truthy
269+
// CG_TRUST_PROXY=1 meant "there is a proxy" and maps onto "xff". Settings win over the env.
270+
export const TRUST_PROXY_MODES = ["", "cloudflare", "xff"];
271+
export function normalizeTrustProxy(value) {
272+
const v = String(value ?? "").trim().toLowerCase();
273+
if (v === "" || v === "0" || v === "false" || v === "no" || v === "off" || v === "none") return "";
274+
if (v === "cloudflare" || v === "cf") return "cloudflare";
275+
if (v === "xff" || v === "x-forwarded-for" || v === "1" || v === "true" || v === "yes" || v === "on") return "xff";
276+
return null; // not a mode
277+
}
278+
export function getTrustProxy() {
279+
const s = getSettings();
280+
if (typeof s.trustProxy === "string") return normalizeTrustProxy(s.trustProxy) ?? "";
281+
return normalizeTrustProxy(process.env.CG_TRUST_PROXY) ?? "";
282+
}
283+
265284
// Opt-in recipients of supplemental Slack button links. Missing/invalid legacy values fail closed.
266285
export const AI_TESTING_USER_ID_RE = /^[UW][A-Z0-9]+$/;
267286
export function getAiTestingUsers() {
@@ -1105,6 +1124,7 @@ export function settingsForApi() {
11051124
composioMcpUrl: s.composioMcpUrl ?? process.env.COMPOSIO_MCP_URL ?? "https://connect.composio.dev/mcp",
11061125
toolboxMcpUrl: s.toolboxMcpUrl ?? process.env.TOOLBOX_MCP_URL ?? "https://www.skillsmanager.uk/toolbox",
11071126
publicUrl: getPublicUrl(),
1127+
trustProxy: getTrustProxy(),
11081128
approvalLinks: getApprovalLinks(),
11091129
aiTestingUsers: getAiTestingUsers(),
11101130
progressView: getProgressView(),

‎src/web/auth.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,11 @@ export function noPasswordLockdown(req, res, next) {
195195
// Which identity the login backoff counts against. Behind a reverse proxy or tunnel every request
196196
// arrives from 127.0.0.1, so keying on the socket puts all remote clients in ONE bucket: an
197197
// attacker's failures lock out the real admin, and distributed guessing looks like one client.
198-
// The forwarding headers fix that and are honoured exactly where they cannot be forged — from a
199-
// loopback socket, which only the proxy in front and this host's own processes can open — or when
200-
// the operator declares a trusted proxy elsewhere on the network (CG_TRUST_PROXY). One helper,
201-
// shared with the approval-link router, so both limiters bucket callers the same way.
198+
// The forwarding header of the proxy the operator DECLARED (`trustProxy`: cloudflare → the
199+
// CF-Connecting-IP it overwrites, xff → the last X-Forwarded-For hop it appended) fixes that; an
200+
// undeclared header is a client's claim and is ignored, and a global failure cap bounds guessing
201+
// either way. One helper, shared with the approval-link router, so both limiters bucket callers
202+
// the same way.
202203
export function loginKey(req) {
203204
return clientKey(req);
204205
}

‎src/web/routes/approve.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ export const APPROVAL_LINK_PRINCIPAL = "link";
4040
// exponential backoff as a failed admin login. A CORRECT token never records a failure, so a
4141
// person retrying their own link is never slowed down. "Per-IP" is per REAL client: these URLs are
4242
// opened from the internet through a loopback tunnel, and keying on the socket would let six bad
43-
// tokens from anywhere lock out every legitimate link (see clientKey in ../security.js).
43+
// tokens from anywhere lock out every legitimate link — once the operator has declared the proxy
44+
// whose header names the client (see clientKey in ../security.js and the `trustProxy` setting).
4445
const DEFAULT_LIMITER = { freeAttempts: 5, baseDelayMs: 1_000, maxDelayMs: 60_000 };
4546

4647
const escapeHtml = (value) =>

0 commit comments

Comments
 (0)