Skip to content

Commit 5c84639

Browse files
mschmickingclaude
andcommitted
Generate the TLS test certificate instead of committing it
A committed private key, even one scoped to localhost with no security value, is flagged by every secret scanner forever, and the false positive has to be re-triaged on each new scan. SonarQube, CodeQL and GitHub secret scanning would all report it. fake-server now generates the pair on first use and caches it under test/fixtures/, which is gitignored. Cold generation costs about 150 ms once; every later run is exactly as fast as reading a committed fixture, so the local speed advantage is preserved. Without openssl on PATH the TLS suite skips rather than fails, so a contributor lacking it still gets a green run — at the cost of not covering the HTTPS login path. Also adds a CodeQL workflow. It cannot run while the repository is private (that needs Advanced Security) but will start on the first push after it goes public. Note the historical blob remains reachable in earlier commits; removing it entirely would need another history rewrite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 41b35b5 commit 5c84639

7 files changed

Lines changed: 125 additions & 95 deletions

File tree

.github/workflows/codeql.yml

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
name: CodeQL
2+
3+
# GitHub's own static analysis. Free on public repositories; on a private repo it
4+
# requires GitHub Advanced Security, so this will simply not run until the
5+
# repository is public.
6+
on:
7+
push:
8+
branches: [main]
9+
pull_request:
10+
branches: [main]
11+
schedule:
12+
# Rules are updated continuously, so a weekly run finds things that did not
13+
# exist as findings when the code was written.
14+
- cron: '0 7 * * 1'
15+
16+
jobs:
17+
analyze:
18+
runs-on: ubuntu-latest
19+
permissions:
20+
security-events: write
21+
contents: read
22+
23+
steps:
24+
- uses: actions/checkout@v4
25+
26+
- uses: github/codeql-action/init@v3
27+
with:
28+
languages: javascript-typescript
29+
# security-extended adds lower-severity rules; worth it for a tool that
30+
# handles credentials and writes files from server-controlled ids.
31+
queries: security-extended
32+
33+
- uses: github/codeql-action/analyze@v3

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,6 @@ dist-test/
1212
.iobroker/
1313

1414
*.log
15+
16+
# TLS test certificate, generated on first test run (never committed)
17+
test/fixtures/*.pem

test/auth-tls.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,16 @@ import { after, afterEach, before, beforeEach, describe, it } from 'node:test';
1616
import assert from 'node:assert/strict';
1717
import * as path from 'node:path';
1818

19-
import { FakeAdminServer } from './fake-server';
19+
import { FakeAdminServer, tlsFixtureAvailable } from './fake-server';
2020
import { getAuthCookie } from '../src/client/auth';
2121
import { UserError } from '../src/types';
2222
import { TempProject, makeTempProject } from './helpers';
2323

2424
const NO_PROMPT = { allowPrompt: false } as const;
2525

26-
describe('getAuthCookie over https with a self-signed certificate', () => {
26+
// openssl generates the certificate on first run; without it these are skipped
27+
// rather than failing, so a contributor without openssl still gets a green suite.
28+
describe('getAuthCookie over https with a self-signed certificate', { skip: tlsFixtureAvailable() ? false : 'openssl not available' }, () => {
2729
let server: FakeAdminServer;
2830
let url: string;
2931
let project: TempProject;

test/fake-server.ts

Lines changed: 71 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* `[3, id, command, args]` and are answered with `[3, id, null, [err, result]]`.
99
*/
1010

11+
import { execFileSync } from 'node:child_process';
1112
import * as fs from 'node:fs';
1213
import * as http from 'node:http';
1314
import * as https from 'node:https';
@@ -16,24 +17,68 @@ import WebSocket from 'ws';
1617
import { IoBrokerObject } from '../src/types';
1718

1819
/**
19-
* Self-signed certificate for the TLS mode, generated with a 20-year lifetime and
20-
* committed under `test/fixtures/`. An ioBroker instance with authentication enabled
21-
* is normally also on HTTPS with exactly this kind of certificate, so the
22-
* `allowSelfSigned` path needs a real TLS handshake to be tested at all.
20+
* Self-signed certificate for the TLS mode.
2321
*
24-
* Resolved against the source tree because the compiled tests live in `dist-test/`
25-
* and the `.pem` files are not copied there.
22+
* An ioBroker instance with authentication enabled is normally also on HTTPS with
23+
* exactly this kind of certificate, so the `allowSelfSigned` path needs a real TLS
24+
* handshake to be tested at all.
25+
*
26+
* The pair is **generated on first use and cached** in `test/fixtures/`, not committed.
27+
* Committing a private key — even a worthless one for localhost — means every future
28+
* secret scanner flags the repository forever. Generating it per run would be slower,
29+
* so it is written once and reused; the files are gitignored.
30+
*
31+
* Resolved against the source tree because the compiled tests live in `dist-test/`.
2632
*/
27-
function fixturePath(name: string): string {
28-
const candidates = [
29-
path.resolve(process.cwd(), 'test', 'fixtures', name),
30-
path.resolve(__dirname, 'fixtures', name),
31-
path.resolve(__dirname, '..', '..', 'test', 'fixtures', name),
32-
];
33-
for (const candidate of candidates) {
33+
function fixturesDir(): string {
34+
for (const candidate of [
35+
path.resolve(process.cwd(), 'test', 'fixtures'),
36+
path.resolve(__dirname, 'fixtures'),
37+
path.resolve(__dirname, '..', '..', 'test', 'fixtures'),
38+
]) {
3439
if (fs.existsSync(candidate)) return candidate;
3540
}
36-
throw new Error(`Could not locate test fixture "${name}"`);
41+
return path.resolve(process.cwd(), 'test', 'fixtures');
42+
}
43+
44+
/** True when a TLS fixture can be produced, i.e. `openssl` is on PATH. */
45+
export function tlsFixtureAvailable(): boolean {
46+
return ensureTlsFixture() !== null;
47+
}
48+
49+
let cachedFixture: { key: Buffer; cert: Buffer } | null | undefined;
50+
51+
function ensureTlsFixture(): { key: Buffer; cert: Buffer } | null {
52+
if (cachedFixture !== undefined) return cachedFixture;
53+
54+
const dir = fixturesDir();
55+
const keyPath = path.join(dir, 'self-signed-key.pem');
56+
const certPath = path.join(dir, 'self-signed-cert.pem');
57+
58+
if (!fs.existsSync(keyPath) || !fs.existsSync(certPath)) {
59+
try {
60+
fs.mkdirSync(dir, { recursive: true });
61+
execFileSync(
62+
'openssl',
63+
[
64+
'req', '-x509', '-newkey', 'rsa:2048', '-nodes',
65+
'-days', '7300',
66+
'-subj', '/CN=localhost',
67+
'-addext', 'subjectAltName=DNS:localhost,IP:127.0.0.1',
68+
'-keyout', keyPath,
69+
'-out', certPath,
70+
],
71+
{ stdio: 'ignore' },
72+
);
73+
} catch {
74+
// No openssl. The TLS suite skips rather than failing the whole run.
75+
cachedFixture = null;
76+
return cachedFixture;
77+
}
78+
}
79+
80+
cachedFixture = { key: fs.readFileSync(keyPath), cert: fs.readFileSync(certPath) };
81+
return cachedFixture;
3782
}
3883

3984
type Frame = [number, (number | null)?, string?, unknown?];
@@ -113,10 +158,9 @@ export class FakeAdminServer {
113158
/** Set to true once a client has replied `[2]` to a server-initiated `[1]` ping. */
114159
pongReceived = false;
115160

116-
/** Binds `port` (default 0 = random free port) and resolves with the bound port. */
117161
/**
118162
* Binds `port` (0 = random free port) and resolves with it. With `tls: true` the
119-
* server speaks HTTPS using the committed self-signed certificate, which is what
163+
* server speaks HTTPS using a locally generated self-signed certificate, which is what
120164
* an auth-enabled ioBroker instance normally looks like.
121165
*/
122166
start(port = 0, opts: { tls?: boolean } = {}): Promise<number> {
@@ -127,15 +171,17 @@ export class FakeAdminServer {
127171
void this.handleHttp(req, res);
128172
};
129173

130-
const httpServer = opts.tls
131-
? https.createServer(
132-
{
133-
key: fs.readFileSync(fixturePath('self-signed-key.pem')),
134-
cert: fs.readFileSync(fixturePath('self-signed-cert.pem')),
135-
},
136-
handler,
137-
)
138-
: http.createServer(handler);
174+
let httpServer: http.Server;
175+
if (opts.tls) {
176+
const fixture = ensureTlsFixture();
177+
if (!fixture) {
178+
reject(new Error('TLS mode needs openssl on PATH to generate a test certificate.'));
179+
return;
180+
}
181+
httpServer = https.createServer({ key: fixture.key, cert: fixture.cert }, handler);
182+
} else {
183+
httpServer = http.createServer(handler);
184+
}
139185
this.httpServer = httpServer;
140186

141187
const wss = new WebSocket.Server({ server: httpServer });

test/fixtures/README.md

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,21 @@
11
# Test fixtures
22

3-
## `self-signed-cert.pem` / `self-signed-key.pem`
3+
## TLS certificate — generated, not committed
44

5-
A throwaway self-signed certificate for `localhost` / `127.0.0.1`, valid until 2046,
6-
used by `test/auth-tls.test.ts` to start the fake Admin server over HTTPS.
5+
`test/auth-tls.test.ts` needs a real TLS handshake, because an ioBroker instance with
6+
authentication enabled is normally also on HTTPS with a self-signed certificate, and
7+
`allowSelfSigned` cannot be exercised against a mock.
78

8-
**This key is deliberately committed and has no security value.** It protects nothing,
9-
is generated for this repository only, and is never used outside an in-process test
10-
server bound to a random loopback port.
9+
`test/fake-server.ts` generates `self-signed-key.pem` / `self-signed-cert.pem` here on
10+
first use and reuses them afterwards. Both are gitignored.
1111

12-
It exists because an ioBroker instance with authentication enabled is normally also on
13-
HTTPS with a self-signed certificate — Admin will not take a password over plain HTTP.
14-
Testing the `allowSelfSigned` path therefore needs a real TLS handshake, not a mock.
12+
**Why generated rather than committed:** a committed private key — even a worthless one
13+
scoped to localhost — is flagged by every secret scanner, forever, and the false positive
14+
has to be re-triaged on each new scan. Generating costs roughly 150 ms once; the cached
15+
pair then makes every later run exactly as fast as a committed fixture would be.
1516

16-
Regenerate with:
17+
Requires `openssl` on PATH. Without it the TLS suite **skips** rather than fails, so a
18+
contributor who lacks it still gets a green run — at the cost of not covering the HTTPS
19+
login path.
1720

18-
```bash
19-
openssl req -x509 -newkey rsa:2048 -nodes -days 7300 \
20-
-subj "/CN=localhost" \
21-
-addext "subjectAltName=DNS:localhost,IP:127.0.0.1" \
22-
-keyout self-signed-key.pem -out self-signed-cert.pem
23-
```
24-
25-
> **Before making this repository public:** GitHub secret scanning flags committed
26-
> private keys, even harmless ones. Either accept the alert and dismiss it as a test
27-
> fixture, or switch to generating the pair at test-setup time (CI runners have
28-
> `openssl`).
21+
To force a fresh pair, delete the two `.pem` files and run the tests again.

test/fixtures/self-signed-cert.pem

Lines changed: 0 additions & 19 deletions
This file was deleted.

test/fixtures/self-signed-key.pem

Lines changed: 0 additions & 28 deletions
This file was deleted.

0 commit comments

Comments
 (0)