Skip to content

Commit 46563ad

Browse files
authored
security(kiloclaw) set internal chat URL as server side variable (#3677)
harden: source kilo-chat internal-call destination from server-only KILO_CHAT_INTERNAL_URL
1 parent a70e028 commit 46563ad

3 files changed

Lines changed: 101 additions & 21 deletions

File tree

apps/web/.env.development.local.example

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ NEXT_PUBLIC_GASTOWN_URL=http://localhost:8803
5959
# @url kilo-chat
6060
NEXT_PUBLIC_KILO_CHAT_URL=http://localhost:8808
6161

62+
# Server-only destination for the cloud → kilo-chat internal call (carries the
63+
# internal API key + prompt). Kept separate from the NEXT_PUBLIC_ var so the
64+
# destination is never browser-exposed. Locally points at the same kilo-chat.
65+
# @url kilo-chat
66+
KILO_CHAT_INTERNAL_URL=http://localhost:8808
67+
6268
# @url event-service
6369
NEXT_PUBLIC_EVENT_SERVICE_URL=ws://localhost:8809
6470

apps/web/src/lib/kiloclaw/kilo-chat-internal-client.test.ts

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
// Env must be set BEFORE importing the module under test so its constants
2-
// resolve to the test values.
2+
// resolve to the test values. KILO_CHAT_INTERNAL_URL is the preferred
3+
// server-only destination; NEXT_PUBLIC_KILO_CHAT_URL is the migration-safe
4+
// fallback.
5+
process.env.KILO_CHAT_INTERNAL_URL = 'https://chat.kiloapps.io';
36
process.env.NEXT_PUBLIC_KILO_CHAT_URL = 'https://chat.kiloapps.io';
47

58
jest.mock('@/lib/config.server', () => ({
@@ -113,29 +116,81 @@ describe('postMessageAsUser (cloud → kilo-chat internal HTTP)', () => {
113116
await expect(postMessageAsUser(VALID_PARAMS)).rejects.toThrow(/unexpected payload/);
114117
});
115118

116-
it('throws when NEXT_PUBLIC_KILO_CHAT_URL is missing', async () => {
117-
const saved = process.env.NEXT_PUBLIC_KILO_CHAT_URL;
119+
it('prefers KILO_CHAT_INTERNAL_URL over NEXT_PUBLIC_KILO_CHAT_URL', async () => {
120+
const savedInternal = process.env.KILO_CHAT_INTERNAL_URL;
121+
const savedPublic = process.env.NEXT_PUBLIC_KILO_CHAT_URL;
122+
// Point the public var somewhere off-allowlist: if it were used, the call
123+
// would be refused. The internal var must win and the fetch must go to it.
124+
process.env.KILO_CHAT_INTERNAL_URL = 'https://chat.kiloapps.io';
125+
process.env.NEXT_PUBLIC_KILO_CHAT_URL = 'https://evil.example.com';
126+
const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue(
127+
jsonResponse({
128+
ok: true,
129+
conversationId: 'conv-1',
130+
messageId: 'msg-1',
131+
conversationCreated: false,
132+
})
133+
);
134+
try {
135+
await postMessageAsUser(VALID_PARAMS);
136+
const [url] = fetchSpy.mock.calls[0]!;
137+
expect(url).toBe('https://chat.kiloapps.io/internal/v1/post-message-as-user');
138+
} finally {
139+
process.env.KILO_CHAT_INTERNAL_URL = savedInternal;
140+
process.env.NEXT_PUBLIC_KILO_CHAT_URL = savedPublic;
141+
}
142+
});
143+
144+
it('falls back to NEXT_PUBLIC_KILO_CHAT_URL with a warning when the internal var is unset', async () => {
145+
const savedInternal = process.env.KILO_CHAT_INTERNAL_URL;
146+
delete process.env.KILO_CHAT_INTERNAL_URL;
147+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
148+
const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue(
149+
jsonResponse({
150+
ok: true,
151+
conversationId: 'conv-1',
152+
messageId: 'msg-1',
153+
conversationCreated: false,
154+
})
155+
);
156+
try {
157+
await postMessageAsUser(VALID_PARAMS);
158+
const [url] = fetchSpy.mock.calls[0]!;
159+
expect(url).toBe('https://chat.kiloapps.io/internal/v1/post-message-as-user');
160+
expect(warnSpy).toHaveBeenCalledWith(
161+
expect.stringMatching(/KILO_CHAT_INTERNAL_URL is not set/)
162+
);
163+
} finally {
164+
process.env.KILO_CHAT_INTERNAL_URL = savedInternal;
165+
}
166+
});
167+
168+
it('throws when neither destination var is configured', async () => {
169+
const savedInternal = process.env.KILO_CHAT_INTERNAL_URL;
170+
const savedPublic = process.env.NEXT_PUBLIC_KILO_CHAT_URL;
171+
delete process.env.KILO_CHAT_INTERNAL_URL;
118172
delete process.env.NEXT_PUBLIC_KILO_CHAT_URL;
119173
try {
120174
await expect(postMessageAsUser(VALID_PARAMS)).rejects.toThrow(
121-
/NEXT_PUBLIC_KILO_CHAT_URL is not configured/
175+
/Neither KILO_CHAT_INTERNAL_URL nor NEXT_PUBLIC_KILO_CHAT_URL is configured/
122176
);
123177
} finally {
124-
process.env.NEXT_PUBLIC_KILO_CHAT_URL = saved;
178+
process.env.KILO_CHAT_INTERNAL_URL = savedInternal;
179+
process.env.NEXT_PUBLIC_KILO_CHAT_URL = savedPublic;
125180
}
126181
});
127182

128183
it('refuses to send the key to an off-allowlist origin (never fetches)', async () => {
129-
const saved = process.env.NEXT_PUBLIC_KILO_CHAT_URL;
130-
process.env.NEXT_PUBLIC_KILO_CHAT_URL = 'https://evil.example.com';
184+
const saved = process.env.KILO_CHAT_INTERNAL_URL;
185+
process.env.KILO_CHAT_INTERNAL_URL = 'https://evil.example.com';
131186
const fetchSpy = jest.spyOn(global, 'fetch');
132187
try {
133188
await expect(postMessageAsUser(VALID_PARAMS)).rejects.toThrow(
134189
/not an allowed kilo-chat origin/
135190
);
136191
expect(fetchSpy).not.toHaveBeenCalled();
137192
} finally {
138-
process.env.NEXT_PUBLIC_KILO_CHAT_URL = saved;
193+
process.env.KILO_CHAT_INTERNAL_URL = saved;
139194
}
140195
});
141196

apps/web/src/lib/kiloclaw/kilo-chat-internal-client.ts

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,21 @@ const POST_MESSAGE_AS_USER_TIMEOUT_MS = 5_000;
2121
* `x-internal-api-key` header that kilo-chat's `internalApiMiddleware`
2222
* timing-safe compares against `INTERNAL_API_SECRET`.
2323
*
24-
* Both env vars are required at runtime:
25-
* - `NEXT_PUBLIC_KILO_CHAT_URL` — already used by the existing public
26-
* client-side kilo-chat token flow; we reuse it for the internal path.
24+
* Two env vars are required at runtime:
25+
* - `KILO_CHAT_INTERNAL_URL` — server-only destination for the internal
26+
* call. Preferred so the key + prompt destination is never sourced from a
27+
* browser-exposed `NEXT_PUBLIC_*` var. Falls back to
28+
* `NEXT_PUBLIC_KILO_CHAT_URL` (with a warning) until the new var is
29+
* provisioned on every project; a follow-up drops the fallback.
2730
* - `INTERNAL_API_SECRET` — shared secret with kilo-chat's Secrets Store
2831
* binding. Already used by other cloud → service integrations.
2932
*/
3033

3134
// Origins the internal API key may be sent to. The destination comes from
32-
// NEXT_PUBLIC_KILO_CHAT_URL (deploy config), so this is defense in depth: a
33-
// misconfigured or tampered value must not be able to forward the key (and the
34-
// prompt) to an unexpected host. `chat.kiloapps.io` is the single deployed
35+
// deploy config (KILO_CHAT_INTERNAL_URL, or the NEXT_PUBLIC_KILO_CHAT_URL
36+
// fallback), so this is defense in depth: a misconfigured or tampered value
37+
// must not be able to forward the key (and the prompt) to an unexpected host.
38+
// `chat.kiloapps.io` is the single deployed
3539
// kilo-chat origin (services/kilo-chat/wrangler.jsonc). Loopback covers local
3640
// dev on any port (KILO_PORT_OFFSET can shift it). Add new deployed origins
3741
// here if kilo-chat ever gains a staging domain.
@@ -42,21 +46,36 @@ function isAllowedKiloChatOrigin(url: URL): boolean {
4246
}
4347

4448
function getKiloChatBaseUrl(): string {
45-
// Read process.env directly rather than importing KILO_CHAT_URL from
46-
// `@/lib/constants`: that constant is marked required at import time, which
47-
// crashes test setups if the var is unset. This server-only client should
48-
// fail loudly only when it is actually called.
49-
const raw = process.env.NEXT_PUBLIC_KILO_CHAT_URL;
49+
// Prefer the server-only KILO_CHAT_INTERNAL_URL so the internal-call
50+
// destination is never sourced from a browser-exposed NEXT_PUBLIC_* var.
51+
// Migration-safe fallback to NEXT_PUBLIC_KILO_CHAT_URL (with a warning) so an
52+
// environment that has not provisioned the new var yet keeps working rather
53+
// than repeating a wrong-config outage; a follow-up removes the fallback once
54+
// KILO_CHAT_INTERNAL_URL is confirmed on every project. Read process.env
55+
// directly rather than importing from `@/lib/constants`: those constants are
56+
// marked required at import time, which crashes test setups if a var is
57+
// unset. This server-only client should fail loudly only when actually called.
58+
let raw = process.env.KILO_CHAT_INTERNAL_URL;
59+
let source = 'KILO_CHAT_INTERNAL_URL';
60+
if (!raw) {
61+
raw = process.env.NEXT_PUBLIC_KILO_CHAT_URL;
62+
source = 'NEXT_PUBLIC_KILO_CHAT_URL';
63+
if (raw) {
64+
console.warn(
65+
'KILO_CHAT_INTERNAL_URL is not set; falling back to NEXT_PUBLIC_KILO_CHAT_URL for the kilo-chat internal call. Set KILO_CHAT_INTERNAL_URL to remove this fallback.'
66+
);
67+
}
68+
}
5069
if (!raw) {
5170
throw new Error(
52-
'NEXT_PUBLIC_KILO_CHAT_URL is not configured, cannot reach kilo-chat internal routes'
71+
'Neither KILO_CHAT_INTERNAL_URL nor NEXT_PUBLIC_KILO_CHAT_URL is configured, cannot reach kilo-chat internal routes'
5372
);
5473
}
5574
let parsed: URL;
5675
try {
5776
parsed = new URL(raw);
5877
} catch {
59-
throw new Error(`NEXT_PUBLIC_KILO_CHAT_URL is not a valid URL: ${raw}`);
78+
throw new Error(`${source} is not a valid URL: ${raw}`);
6079
}
6180
if (!isAllowedKiloChatOrigin(parsed)) {
6281
throw new Error(

0 commit comments

Comments
 (0)