Skip to content

Commit da5f91c

Browse files
committed
test(cli,hono): pin the TLS listener, the derived origin, and the no-flag ablation
- `adapter-tls-listener.test.ts` drives a real ephemeral TLS socket with a certificate the test mints into a tempdir, and drives the two legs against each other: the https listener refuses a plain-http request and the plain listener refuses a TLS one, so neither leg can pass by being broken. - `dev-tls-contract.test.ts` pins the three answers of the flag pair, the refusals, and — negatively — that no flag description and no line of the module mentions generating a certificate or trusting one, with an anti-vacuity case proving the same scan reads words that are there. - `dev-mcp-connect-hint-origin.test.ts` gains the https acceptance (banner row and all three hint lines, `OS_AUTH_URL` unset) and an ablation leg: the boot that passes no protocol at all and the boot that passes `http` must be byte-identical, and both must differ from the https leg. - `serve-auth-base-url-diagnostic.test.ts` pins that the listener protocol reaches the built-in tail and stops there — every configured value keeps winning, an `http://` one included. - `serve-bound-port-publication.test.ts` gains the same ablation for the state file and the IPC message, and its two source pins are updated to the spellings this change moved. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2b17dec commit da5f91c

5 files changed

Lines changed: 720 additions & 5 deletions

File tree

‎packages/cli/src/commands/dev-mcp-connect-hint-origin.test.ts‎

Lines changed: 130 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,28 @@ describe('os dev MCP connect hint — origin (#16734)', () => {
8989
* child's banner (its own call site's expression, verbatim) and then the
9090
* parent's connect hint, both told the port the server ACTUALLY bound.
9191
*/
92-
const boot = (boundPort: number, name = 'hotcrm') => {
92+
const boot = (boundPort: number, name = 'hotcrm', boundProtocol: 'http' | 'https' = 'http') => {
93+
printServerReady({
94+
...bannerOpts,
95+
externalBaseOrigin: resolveAuthBaseUrl(boundPort, boundProtocol).baseOrigin,
96+
});
97+
printMcpConnectHint({ boundPort, name, boundProtocol });
98+
return lines.join('\n');
99+
};
100+
101+
/**
102+
* The same boot with the protocol argument WITHHELD from both printers — the
103+
* expression this file carried before `--cert`/`--key` existed, character for
104+
* character.
105+
*
106+
* ⭐ This is the ABLATION LEG for acceptance 2 (「无 flag 时逐字节等于今天」,
107+
* #16804). The pins below drive it beside {@link boot}'s no-flag case and
108+
* require the two to be byte-identical, so the claim "nothing changes without
109+
* the flags" is measured rather than asserted: it would go red the day the
110+
* derived protocol leaked into the default, and it cannot pass by both legs
111+
* being broken in the same direction.
112+
*/
113+
const bootWithoutProtocolArg = (boundPort: number, name = 'hotcrm') => {
93114
printServerReady({ ...bannerOpts, externalBaseOrigin: resolveAuthBaseUrl(boundPort).baseOrigin });
94115
printMcpConnectHint({ boundPort, name });
95116
return lines.join('\n');
@@ -203,10 +224,117 @@ describe('os dev MCP connect hint — origin (#16734)', () => {
203224
});
204225
});
205226

227+
// ── #16804 ─ a TLS listener, and the three surfaces that follow it ──────
228+
describe('under --cert/--key the derived origin is https, and everything follows', () => {
229+
it('acceptance 1: with OS_AUTH_URL UNSET, banner and hint both give https', () => {
230+
// Every chain variable is deleted by `beforeEach`, so the only thing
231+
// that can produce `https` here is the listener protocol reaching the
232+
// resolver's built-in tail.
233+
const output = boot(3000, 'hotcrm', 'https');
234+
235+
expect(output).toContain('MCP: https://localhost:3000/api/v1/mcp');
236+
expect(output).toContain('Endpoint https://localhost:3000/api/v1/mcp');
237+
expect(output).toContain('Skill https://localhost:3000/api/v1/mcp/skill');
238+
expect(output).toContain(
239+
'Connect claude mcp add --transport http hotcrm https://localhost:3000/api/v1/mcp',
240+
);
241+
// The plain-http address must not appear anywhere in a TLS boot's output:
242+
// it is the one address a client on this port cannot reach.
243+
expect(output).not.toContain('http://localhost:3000');
244+
expect(mcpOrigins(output)).toEqual(['https://localhost:3000']);
245+
});
246+
247+
it("follows dev's auto-shifted port under TLS too", () => {
248+
expect(mcpOrigins(boot(3001, 'hotcrm', 'https'))).toEqual(['https://localhost:3001']);
249+
});
250+
251+
it('acceptance 3: `OS_AUTH_URL` still WINS over the derived https origin', () => {
252+
// The override direction that matters in practice: a developer
253+
// terminating TLS locally but reached through a tunnel on another host.
254+
process.env.OS_AUTH_URL = 'https://tunnel.example.com';
255+
expect(mcpOrigins(boot(3000, 'hotcrm', 'https'))).toEqual(['https://tunnel.example.com']);
256+
});
257+
258+
it('acceptance 3, the awkward direction: an http OS_AUTH_URL wins as well', () => {
259+
// ⛔ Deliberately NOT "upgraded" to https. `OS_AUTH_URL` names where the
260+
// deployment is REACHED — behind a TLS-terminating proxy that forwards
261+
// plain http, or in a test harness, that is a deliberate statement about
262+
// a different hop, and a default has no standing to overrule it.
263+
process.env.OS_AUTH_URL = 'http://proxied.example.com';
264+
expect(mcpOrigins(boot(3000, 'hotcrm', 'https'))).toEqual(['http://proxied.example.com']);
265+
});
266+
267+
it('the rest of the configured chain keeps its precedence under TLS', () => {
268+
process.env.OS_BASE_URL = 'https://base.example.com';
269+
expect(mcpOrigins(boot(3000, 'hotcrm', 'https'))).toEqual(['https://base.example.com']);
270+
});
271+
272+
it('an unusable value stays unusable — TLS does not manufacture an origin', () => {
273+
process.env.OS_AUTH_URL = '';
274+
const output = boot(3000, 'hotcrm', 'https');
275+
expect(mcpOrigins(output)).toEqual([]);
276+
expect(output).not.toContain('claude mcp add');
277+
expect(output).not.toContain('https://localhost:3000');
278+
});
279+
});
280+
281+
// ── #16804 ─ acceptance 2, as an ABLATION rather than a claim ──────────
282+
describe('without the flags the output is byte-for-byte what it was', () => {
283+
it('the no-flag boot equals the boot that never passes a protocol at all', () => {
284+
// Leg A: the call shape this file used before `--cert`/`--key` existed.
285+
const before = bootWithoutProtocolArg(3000, 'my-app');
286+
lines.length = 0;
287+
// Leg B: the same boot through today's call shape, no flags given.
288+
const after = boot(3000, 'my-app', 'http');
289+
290+
expect(after).toBe(before);
291+
// And the byte the two legs are about: still plain http, on the bound port.
292+
expect(mcpOrigins(after)).toEqual(['http://localhost:3000']);
293+
});
294+
295+
it('the two legs also agree on an auto-shifted port and an ephemeral one', () => {
296+
for (const port of [3001, 45064]) {
297+
lines.length = 0;
298+
const before = bootWithoutProtocolArg(port, 'my-app');
299+
lines.length = 0;
300+
expect(boot(port, 'my-app', 'http')).toBe(before);
301+
}
302+
});
303+
304+
it('and the legs DISCRIMINATE — the https leg differs from both', () => {
305+
// Without this, two legs that both silently produced nothing would pass.
306+
const plain = bootWithoutProtocolArg(3000, 'my-app');
307+
lines.length = 0;
308+
const tls = boot(3000, 'my-app', 'https');
309+
310+
expect(tls).not.toBe(plain);
311+
expect(mcpOrigins(plain)).toEqual(['http://localhost:3000']);
312+
expect(mcpOrigins(tls)).toEqual(['https://localhost:3000']);
313+
});
314+
});
315+
206316
// ── What only the source can say ────────────────────────────────────────
207317
describe('the call site feeds the printer the bound port, and nothing else', () => {
208318
it('hands `printMcpConnectHint` the ACTUALLY BOUND port', () => {
209-
expect(DEV_SOURCE).toContain('printMcpConnectHint({ boundPort: actual,');
319+
expect(DEV_SOURCE).toContain('printMcpConnectHint({');
320+
expect(DEV_SOURCE).toContain('boundPort: actual,');
321+
});
322+
323+
it('hands it the protocol it FORWARDED, not one derived a second time (#16804)', () => {
324+
// The parent's `boundProtocol` comes from the same `tlsIntent` that built
325+
// the child's argv, so the scheme the hint prints and the scheme the
326+
// child bound cannot part company. A second `resolveDevTlsIntent` call
327+
// here would be a second reader, free to disagree.
328+
expect(DEV_SOURCE).toContain('boundProtocol,');
329+
expect(DEV_SOURCE).toContain('const boundProtocol: ListenerProtocol = listenerProtocol(tlsIntent);');
330+
expect(DEV_SOURCE.match(/resolveDevTlsIntent\(/g) ?? []).toHaveLength(1);
331+
});
332+
333+
it('forwards the cert/key PATHS to the serve child through the shared contract', () => {
334+
expect(DEV_SOURCE).toContain('...devTlsChildArgs(tlsIntent),');
335+
// ⛔ and never the bytes: one reader of the file, one owner of the refusal.
336+
expect(DEV_SOURCE).not.toContain('readDevTlsMaterial');
337+
expect(DEV_SOURCE).not.toContain('readFileSync(flags.cert');
210338
});
211339

212340
it('builds no address out of the listening message any more', () => {

‎packages/cli/src/commands/serve-auth-base-url-diagnostic.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,71 @@ describe('resolveAuthBaseUrl — precedence (pre-existing behaviour, unchanged)'
135135
});
136136
});
137137

138+
/**
139+
* #16804 — the built-in default follows the LISTENER, and nothing else does.
140+
*
141+
* `os dev --cert … --key …` terminates TLS in the dev process, after which
142+
* `http://localhost:<port>` is an address no client can reach. The tail is the
143+
* one link in the chain nobody configured — it is this process describing its
144+
* own socket — so it is the only link that moves.
145+
*/
146+
describe('resolveAuthBaseUrl — the listener protocol reaches the TAIL and stops there', () => {
147+
it('derives https://localhost:<port> for a TLS listener with nothing else set', () => {
148+
expect(resolveAuthBaseUrl(3000, 'https')).toEqual({
149+
value: 'https://localhost:3000',
150+
source: null,
151+
baseOrigin: 'https://localhost:3000',
152+
});
153+
});
154+
155+
it('ABLATION: the omitted argument and an explicit `http` are the same call', () => {
156+
// Acceptance 2 of the card — 「无 flag时逐字节等于今天」 — measured on the
157+
// resolver rather than claimed. Driven for a plain port, dev's shifted
158+
// port and an ephemeral one, because the tail interpolates the port.
159+
for (const port of [3000, 3001, 45064]) {
160+
expect(resolveAuthBaseUrl(port)).toEqual(resolveAuthBaseUrl(port, 'http'));
161+
// …and the two legs DISCRIMINATE, so the equality above is a reading.
162+
expect(resolveAuthBaseUrl(port)).not.toEqual(resolveAuthBaseUrl(port, 'https'));
163+
}
164+
});
165+
166+
it('⛔ a CONFIGURED value is never rewritten — OS_AUTH_URL wins under TLS', () => {
167+
process.env.OS_AUTH_URL = 'https://tunnel.example.com';
168+
const r = resolveAuthBaseUrl(3000, 'https');
169+
expect(r.value).toBe('https://tunnel.example.com');
170+
expect(r.source).toBe('OS_AUTH_URL');
171+
});
172+
173+
it('⛔ including an http one — the scheme of a configured value is not upgraded', () => {
174+
// The direction a "helpful" implementation gets wrong. `OS_AUTH_URL` names
175+
// where the deployment is REACHED; behind a proxy that forwards plain http
176+
// to this TLS listener, `http://…` is the operator's deliberate statement
177+
// about a different hop, and a default has no standing to overrule it.
178+
process.env.OS_AUTH_URL = 'http://proxied.example.com';
179+
expect(resolveAuthBaseUrl(3000, 'https').value).toBe('http://proxied.example.com');
180+
expect(resolveAuthBaseUrl(3000, 'https').baseOrigin).toBe('http://proxied.example.com');
181+
});
182+
183+
it('⛔ nor the legacy name, nor OS_BASE_URL', () => {
184+
process.env.BETTER_AUTH_URL = 'http://legacy.example.com';
185+
expect(resolveAuthBaseUrl(3000, 'https').value).toBe('http://legacy.example.com');
186+
187+
delete process.env.BETTER_AUTH_URL;
188+
process.env.OS_BASE_URL = 'http://base.example.com';
189+
expect(resolveAuthBaseUrl(3000, 'https').value).toBe('http://base.example.com');
190+
});
191+
192+
it('⛔ a set-but-empty value stays unusable — TLS manufactures no origin', () => {
193+
// The tail is not reached at all here (`??` skips only unset values), so a
194+
// TLS listener must not resurrect it. `baseOrigin: null` means the printers
195+
// say nothing rather than guessing, and that is unchanged.
196+
process.env.OS_AUTH_URL = '';
197+
const r = resolveAuthBaseUrl(3000, 'https');
198+
expect(r.value).toBe('');
199+
expect(r.baseOrigin).toBeNull();
200+
});
201+
});
202+
138203
describe('resolveAuthBaseUrl — a usable base URL stays silent', () => {
139204
it.each([
140205
['OS_AUTH_URL', 'https://app.example.com'],

‎packages/cli/src/commands/serve-bound-port-publication.test.ts‎

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ describe('#13062 all THREE channels publish that one number', () => {
333333
expect(
334334
SERVE,
335335
'the banner no longer resolves its origin from `boundPort`',
336-
).toContain('externalBaseOrigin: resolveAuthBaseUrl(boundPort).baseOrigin');
336+
).toContain('externalBaseOrigin: resolveAuthBaseUrl(boundPort, boundProtocol).baseOrigin');
337337
});
338338

339339
it('the ONE wiring site hands the seam the BOUND port, never the requested one', () => {
@@ -343,14 +343,63 @@ describe('#13062 all THREE channels publish that one number', () => {
343343
expect(
344344
SERVE,
345345
'the publish site no longer hands `publishBoundPort` the resolved bound port',
346-
).toContain('publishBoundPort(boundPort, runtimeBoundPortChannels(printBanner));');
346+
).toContain('publishBoundPort(boundPort, runtimeBoundPortChannels(printBanner), boundProtocol);');
347347
// Exactly two mentions in CODE: the declaration and that single call.
348348
expect(
349349
SERVE.match(/publishBoundPort\(/g) ?? [],
350350
'a second publish site can disagree with the first — that is the #13062 defect returning',
351351
).toHaveLength(2);
352352
});
353353

354+
describe('#16804 — the published url names the scheme the socket SPEAKS', () => {
355+
it('plain http when nothing asked for TLS — today\'s bytes, unchanged', () => {
356+
const seen: Array<{ port: number; url: string }> = [];
357+
publishBoundPort(45070, {
358+
writeRuntimeState: (published) => { seen.push(published); },
359+
announceListening: (message) => { seen.push({ port: message.port, url: message.url }); },
360+
printBanner: () => { /* not under test here */ },
361+
});
362+
expect(seen).toEqual([
363+
{ port: 45070, url: 'http://localhost:45070' },
364+
{ port: 45070, url: 'http://localhost:45070' },
365+
]);
366+
});
367+
368+
it('https on BOTH channels under a TLS listener — an ABLATION beside the leg above', () => {
369+
// Both consumers of this url OPEN it: the runtime state file is what an
370+
// external supervisor dials, the IPC message is what the `os dev` parent
371+
// learns the server from. Under `--cert`/`--key` a hardcoded `http://`
372+
// hands both an address that answers a TLS handshake error.
373+
const seen: Array<{ port: number; url: string }> = [];
374+
publishBoundPort(45071, {
375+
writeRuntimeState: (published) => { seen.push(published); },
376+
announceListening: (message) => { seen.push({ port: message.port, url: message.url }); },
377+
printBanner: () => { /* not under test here */ },
378+
}, 'https');
379+
expect(seen).toEqual([
380+
{ port: 45071, url: 'https://localhost:45071' },
381+
{ port: 45071, url: 'https://localhost:45071' },
382+
]);
383+
});
384+
385+
it('the default parameter IS the old behaviour — omitted and `http` agree', () => {
386+
const capture = (protocol?: 'http' | 'https') => {
387+
const out: string[] = [];
388+
const channels = {
389+
writeRuntimeState: (published: { port: number; url: string }) => { out.push(published.url); },
390+
announceListening: (message: { url: string }) => { out.push(message.url); },
391+
printBanner: () => { /* not under test here */ },
392+
};
393+
if (protocol === undefined) publishBoundPort(45072, channels);
394+
else publishBoundPort(45072, channels, protocol);
395+
return out;
396+
};
397+
expect(capture()).toEqual(capture('http'));
398+
// …and the legs discriminate, so agreeing is a reading and not a vacuum.
399+
expect(capture()).not.toEqual(capture('https'));
400+
});
401+
});
402+
354403
it('⛔ and NONE of the three has drifted back onto the requested port', () => {
355404
// The card's own instruction: three outputs of one defect, and repairing
356405
// one leaves two lying in a place nobody thinks to look next time.
@@ -389,7 +438,13 @@ describe('#13062 all THREE channels publish that one number', () => {
389438
// The positive control for the negatives above: `port` has not been
390439
// globally renamed, so `not.toContain('port: Number(port)')` is a
391440
// measurement rather than a consequence of the variable disappearing.
392-
expect(SERVE).toContain('new HonoServerPlugin({ port })');
441+
//
442+
// #16804 widened the construction literal to carry `tls` beside `port`;
443+
// what this control needs is that `port` still reaches the transport
444+
// under its own name, so it reads the construction site rather than one
445+
// formatting of it.
446+
expect(SERVE).toContain('new HonoServerPlugin({');
447+
expect(SERVE.slice(SERVE.indexOf('new HonoServerPlugin({'))).toMatch(/^new HonoServerPlugin\(\{\s*\n\s*port,/);
393448
expect(SERVE).toContain('port = await getAvailablePort(requestedPort)');
394449
});
395450
});

0 commit comments

Comments
 (0)