Skip to content

Commit 07ada65

Browse files
mschmickingclaude
andauthored
fix: address the two code scanning alerts (#25)
Co-authored-by: Maurice Schmicking <maurice.schmicking@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 84dc84a commit 07ada65

3 files changed

Lines changed: 112 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 {

test/fake-server.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,20 @@ export interface RecordedRequest {
122122
body: string;
123123
}
124124

125+
/**
126+
* Keys that must never be copied by `deepMerge`. `src` arrives as parsed JSON off the
127+
* websocket, so an object id like `__proto__` would otherwise reach through the merge
128+
* and rewrite the prototype instead of setting a property.
129+
*/
130+
const UNSAFE_MERGE_KEYS = new Set(['__proto__', 'constructor', 'prototype']);
131+
125132
/** Deep-merge `src` into `dest`, mirroring the real server's `extendObject` behaviour. */
126133
function deepMerge(
127134
dest: Record<string, unknown>,
128135
src: Record<string, unknown>,
129136
): Record<string, unknown> {
130137
for (const key of Object.keys(src)) {
138+
if (UNSAFE_MERGE_KEYS.has(key)) continue;
131139
const srcVal = src[key];
132140
const destVal = dest[key];
133141
if (

0 commit comments

Comments
 (0)