Skip to content

Commit ee95fcf

Browse files
mschmickingclaude
andcommitted
fix(types): do not overwrite javascript.d.ts with whatever answered
`res.ok` is true for any 200, including the HTML login page a captive portal or a corporate proxy serves. That page was written straight over a working javascript.d.ts, and the damage surfaces later as every script in the editor lighting up with TypeScript errors -- with nothing pointing back at `iob-sync types`. Check three things before the write: the announced content-length, the actual body size, and that the body contains a declaration at all. Each throws into the existing catch, which already warns and keeps the copy on disk, so a portal response now leaves a good cached file intact. The size cap bounds what reaches the disk, not what reaches memory -- res.text() has already buffered the body by then. Bounding that too means streaming the response, which is more than a hardcoded raw.githubusercontent.com URL warrants. The download had no coverage before; it now has the happy path, the portal case, and the oversize case, against a stubbed fetch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c3746b4 commit ee95fcf

2 files changed

Lines changed: 104 additions & 1 deletion

File tree

src/commands/types.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,23 @@ import { Logger } from '../types';
2222
const JAVASCRIPT_DTS_URL =
2323
'https://raw.githubusercontent.com/ioBroker/ioBroker.javascript/refs/heads/master/src/lib/javascript.d.ts';
2424

25+
/**
26+
* Upper bound for the download. The real declaration file is well under 200 KB;
27+
* anything past this is not it, and an unbounded response body has no business
28+
* being buffered and then written into someone's project.
29+
*/
30+
const MAX_DTS_BYTES = 2 * 1024 * 1024;
31+
32+
/**
33+
* A captive portal, a corporate proxy, or a GitHub error page answers 200 with HTML,
34+
* and `res.ok` cannot tell that from the real file. Writing a login page over a
35+
* working javascript.d.ts breaks every script in the editor and presents as a
36+
* TypeScript problem, so check what arrived before it goes near the disk.
37+
*/
38+
function looksLikeDeclarationFile(text: string): boolean {
39+
return /^\s*declare\s/m.test(text);
40+
}
41+
2542
const GLOBAL_DTS_CONTENT = `export {};
2643
declare global {
2744
function require(library: string): any;
@@ -122,7 +139,17 @@ export async function setupTypes(
122139
if (!res.ok) {
123140
throw new Error(`HTTP ${res.status}`);
124141
}
142+
const declared = Number(res.headers.get('content-length'));
143+
if (Number.isFinite(declared) && declared > MAX_DTS_BYTES) {
144+
throw new Error(`the response announces ${Math.round(declared / 1024)} KB`);
145+
}
125146
const text = await res.text();
147+
if (text.length > MAX_DTS_BYTES) {
148+
throw new Error(`the response is ${Math.round(text.length / 1024)} KB`);
149+
}
150+
if (!looksLikeDeclarationFile(text)) {
151+
throw new Error('the response is not a TypeScript declaration file');
152+
}
126153
await fs.writeFile(dtsPath, text, 'utf8');
127154
log.info(`Downloaded javascript.d.ts (${Math.round(text.length / 1024)} KB)`);
128155
} catch (err) {

test/commands-types.test.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
* module semantics per file (no phantom name collisions) and a module target that
77
* permits top-level await.
88
*
9-
* The download is not exercised — it needs the network. `--offline` covers the rest.
9+
* The download is exercised against a stubbed `fetch` rather than the network: what
10+
* matters is that a response which is not the declaration file never reaches the disk.
11+
* `--offline` covers the rest.
1012
*/
1113

1214
import { describe, it } from 'node:test';
@@ -22,6 +24,19 @@ async function readTsconfig(root: string, scriptRoot: string): Promise<Record<st
2224
return JSON.parse(raw) as Record<string, never>;
2325
}
2426

27+
/** Run `fn` with `fetch` replaced by one that answers every call with `res`. */
28+
async function withStubbedFetch(res: Response, fn: () => Promise<void>): Promise<void> {
29+
const real = globalThis.fetch;
30+
globalThis.fetch = () => Promise.resolve(res.clone());
31+
try {
32+
await fn();
33+
} finally {
34+
globalThis.fetch = real;
35+
}
36+
}
37+
38+
const DTS_PATH = ['.iobroker', 'types', 'javascript.d.ts'];
39+
2540
describe('types', () => {
2641
it('writes a config that gives each script its own scope', async () => {
2742
const project = await makeTempProject();
@@ -96,6 +111,67 @@ describe('types', () => {
96111
}
97112
});
98113

114+
it('writes the declaration file when the download is the real thing', async () => {
115+
const project = await makeTempProject();
116+
try {
117+
const dts = 'declare global {\n function log(msg: string): void;\n}\n';
118+
const { log } = makeCapturingLogger();
119+
await withStubbedFetch(new Response(dts, { status: 200 }), async () => {
120+
await setupTypes(project.root, 'scripts', {}, log);
121+
});
122+
123+
assert.equal(await fs.readFile(path.join(project.root, ...DTS_PATH), 'utf8'), dts);
124+
} finally {
125+
await project.cleanup();
126+
}
127+
});
128+
129+
it('leaves a working copy alone when a portal answers 200 with a login page', async () => {
130+
const project = await makeTempProject();
131+
try {
132+
const good = 'declare global {\n function log(msg: string): void;\n}\n';
133+
const dtsPath = path.join(project.root, ...DTS_PATH);
134+
await fs.mkdir(path.dirname(dtsPath), { recursive: true });
135+
await fs.writeFile(dtsPath, good, 'utf8');
136+
137+
const portal = '<!doctype html><html><body>Sign in to continue</body></html>';
138+
const { log, captured } = makeCapturingLogger();
139+
await withStubbedFetch(new Response(portal, { status: 200 }), async () => {
140+
await setupTypes(project.root, 'scripts', {}, log);
141+
});
142+
143+
assert.equal(await fs.readFile(dtsPath, 'utf8'), good, 'the HTML must not overwrite it');
144+
assert.ok(
145+
captured.warn.some((l) => /not a TypeScript declaration file/i.test(l)),
146+
`expected a warning about the response, got ${JSON.stringify(captured.warn)}`,
147+
);
148+
} finally {
149+
await project.cleanup();
150+
}
151+
});
152+
153+
it('refuses a body far too large to be the declaration file', async () => {
154+
const project = await makeTempProject();
155+
try {
156+
const huge = `declare global {}\n${'x'.repeat(3 * 1024 * 1024)}`;
157+
const { log, captured } = makeCapturingLogger();
158+
await withStubbedFetch(new Response(huge, { status: 200 }), async () => {
159+
await setupTypes(project.root, 'scripts', {}, log);
160+
});
161+
162+
await assert.rejects(
163+
() => fs.access(path.join(project.root, ...DTS_PATH)),
164+
'nothing should have been written',
165+
);
166+
assert.ok(
167+
captured.warn.some((l) => l.includes('KB')),
168+
`expected a warning naming the size, got ${JSON.stringify(captured.warn)}`,
169+
);
170+
} finally {
171+
await project.cleanup();
172+
}
173+
});
174+
99175
it('says what is missing rather than failing when offline and nothing is cached', async () => {
100176
const project = await makeTempProject();
101177
try {

0 commit comments

Comments
 (0)