Skip to content

Commit 8de8cbd

Browse files
fix: address Copilot review 3 on PR #7
Four follow-ups from the third pass: 1. Missing-value validation for flags that take an argument. `jspod --host` (or --port, --root) used to read undefined off the args array and crash with a cryptic TypeError on the next operation. Now exits cleanly with "Missing value for --host" and a pointer to --help. 2. IPv6 zone identifiers (e.g. fe80::1%lo0) are now rejected at CLI parse time with a clear error. The previous %25-encoding attempt was correct per RFC 6874 but Node's WHATWG URL parser doesn't accept zone IDs regardless — any URL we built (banner, auto-open, readiness probe) would be unparseable. Failing fast beats shipping silently broken auto-open. 3. TOKEN_SECRET now auto-generates a 48-byte random secret on first run and persists it at <root>/.token-secret with mode 0600. Subsequent restarts read the same secret so sessions and refresh tokens survive process bounces. Per-data-dir, so different pods on the same machine get different secrets. The previous static fallback ('jspod-default-secret-change-in-production') was remotely exploitable on any non-loopback bind — anyone who knew the string could forge JWTs. Env TOKEN_SECRET still wins when set, for operator-managed deployments. 4. README: replaced the literal fallback secret string with accurate docs about auto-generation, persistence path, file mode, and the env override use-case (operator-managed deployments / rotation / secret managers). Refs #1 #3 #6
1 parent f6458f2 commit 8de8cbd

2 files changed

Lines changed: 61 additions & 10 deletions

File tree

README.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ Options:
9191
### Environment Variables
9292

9393
```bash
94-
# Set JWT secret (recommended for production)
95-
export TOKEN_SECRET="your-secret-key-here"
94+
# JWT secret — auto-generated and persisted at <root>/.token-secret on
95+
# first run. Override here only for operator-managed deployments.
96+
export TOKEN_SECRET="$(openssl rand -base64 32)"
9697

9798
# Set environment
9899
export NODE_ENV="production"
@@ -105,7 +106,7 @@ jspod
105106

106107
**⚠️ Important**: Before deploying to production:
107108

108-
1. **Set TOKEN_SECRET**
109+
1. **TOKEN_SECRET** is auto-generated on first run and persisted at `<root>/.token-secret` (mode 0600). For operator-managed deployments — secret rotation, distributed setups, secret managers — set it explicitly via env:
109110
```bash
110111
export TOKEN_SECRET="$(openssl rand -base64 32)"
111112
```
@@ -201,8 +202,9 @@ Under the hood, jspod runs JavaScriptSolidServer with these options:
201202
host: '127.0.0.1', // Localhost-only by default (rung-1 credentials)
202203
root: './pod-data', // Local data directory
203204
multiuser: false, // Single pod per server
204-
TOKEN_SECRET: 'jspod-default-secret-change-in-production'
205-
// Static fallback. Override via env for any non-local use.
205+
TOKEN_SECRET: (per-pod) // Auto-generated on first run and persisted at
206+
// <root>/.token-secret (mode 0600). Override via
207+
// the TOKEN_SECRET env var for operator control.
206208
}
207209
```
208210

index.js

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,19 @@ import { spawn } from 'child_process';
99
import { fileURLToPath } from 'url';
1010
import { dirname, join, delimiter } from 'path';
1111
import chalk from 'chalk';
12-
import { existsSync, mkdirSync, readFileSync } from 'fs';
12+
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs';
13+
import { randomBytes } from 'crypto';
1314

1415
const __dirname = dirname(fileURLToPath(import.meta.url));
1516
const pkg = JSON.parse(readFileSync(join(__dirname, 'package.json'), 'utf8'));
1617

1718
// Build a browser-friendly URL from a host/port pair. Normalizes wildcard
1819
// addresses (0.0.0.0, ::) to localhost and brackets IPv6 literals so the
1920
// result is always a valid URL the user (and the browser) can open.
21+
// IPv6 zone identifiers (e.g. `fe80::1%lo0`) are rejected at CLI parse
22+
// time — the WHATWG URL spec doesn't support them, so any URL we built
23+
// with one would be unparseable by Node and by the browser regardless
24+
// of `%` encoding.
2025
function formatUrl(host, port) {
2126
if (host === '0.0.0.0' || host === '::' || host === '*') {
2227
return `http://localhost:${port}`;
@@ -50,19 +55,42 @@ const RUNG_1_USERNAME = 'me';
5055
const RUNG_1_PASSWORD = process.env.JSS_SINGLE_USER_PASSWORD || 'me';
5156
const RUNG_1_PASSWORD_FROM_ENV = !!process.env.JSS_SINGLE_USER_PASSWORD;
5257

58+
// Require a value after a value-taking flag. Without this guard, a stray
59+
// `jspod --host` (no value) reads `undefined` from args[++i] and the next
60+
// .replace() call throws a cryptic TypeError. Friendlier to error early
61+
// and tell the user what's missing.
62+
function requireValue(flag, value) {
63+
if (value === undefined) {
64+
console.error(chalk.red(`✗ Missing value for ${flag}`));
65+
console.error(chalk.dim('Use --help for usage information'));
66+
process.exit(1);
67+
}
68+
return value;
69+
}
70+
5371
for (let i = 0; i < args.length; i++) {
5472
const arg = args[i];
5573

5674
if (arg === '--port' || arg === '-p') {
57-
options.port = parseInt(args[++i], 10);
75+
options.port = parseInt(requireValue(arg, args[++i]), 10);
5876
} else if (arg === '--host' || arg === '-h') {
5977
// Strip optional brackets from IPv6 literals so a user-friendly
6078
// `--host [::1]` paste-in stays canonical. formatUrl re-adds the
6179
// brackets where they belong in URLs; the raw host going to JSS
6280
// and to comparisons remains the unbracketed literal.
63-
options.host = args[++i].replace(/^\[|\]$/g, '');
81+
const rawHost = requireValue(arg, args[++i]).replace(/^\[|\]$/g, '');
82+
// Reject IPv6 zone identifiers — WHATWG URL spec doesn't support
83+
// them, so any URL we built (banner, browser auto-open, readiness
84+
// probe) would be unparseable. Better to fail fast with a clear
85+
// message than to ship a broken auto-open silently.
86+
if (rawHost.includes('%')) {
87+
console.error(chalk.red(`✗ IPv6 zone identifiers are not supported: ${rawHost}`));
88+
console.error(chalk.dim('Bind to a non-zoned address (e.g. ::1, 127.0.0.1, or your LAN IP) instead.'));
89+
process.exit(1);
90+
}
91+
options.host = rawHost;
6492
} else if (arg === '--root' || arg === '-r') {
65-
options.root = args[++i];
93+
options.root = requireValue(arg, args[++i]);
6694
} else if (arg === '--multiuser') {
6795
options.multiuser = true;
6896
} else if (arg === '--no-auth') {
@@ -115,6 +143,27 @@ if (!existsSync(options.root)) {
115143
mkdirSync(options.root, { recursive: true });
116144
}
117145

146+
// Resolve the JWT signing secret. Priority:
147+
// 1. TOKEN_SECRET env var (operator-controlled)
148+
// 2. Persisted random secret at <root>/.token-secret (generated on
149+
// first run, mode 0600). Same data dir always produces the same
150+
// effective secret across restarts — sessions and refresh tokens
151+
// survive process bounces.
152+
// Generating per-data-dir avoids the previous footgun of a hardcoded
153+
// fallback string that anyone could use to forge JWTs against a
154+
// non-loopback deployment.
155+
function resolveTokenSecret(rootDir) {
156+
if (process.env.TOKEN_SECRET) return process.env.TOKEN_SECRET;
157+
const secretFile = join(rootDir, '.token-secret');
158+
if (existsSync(secretFile)) {
159+
return readFileSync(secretFile, 'utf8').trim();
160+
}
161+
const secret = randomBytes(48).toString('base64');
162+
writeFileSync(secretFile, secret, { mode: 0o600 });
163+
return secret;
164+
}
165+
const tokenSecret = resolveTokenSecret(options.root);
166+
118167
// Display startup banner
119168
console.log(chalk.cyan(`
120169
╔═══════════════════════════════════════════════════════════════════╗
@@ -231,7 +280,7 @@ const jss = spawn('jss', jssArgs, {
231280
env: {
232281
...process.env,
233282
PATH: `${join(__dirname, 'node_modules', '.bin')}${delimiter}${process.env.PATH}`,
234-
TOKEN_SECRET: process.env.TOKEN_SECRET || 'jspod-default-secret-change-in-production',
283+
TOKEN_SECRET: tokenSecret,
235284
NODE_ENV: process.env.NODE_ENV || 'development'
236285
}
237286
});

0 commit comments

Comments
 (0)