-
Notifications
You must be signed in to change notification settings - Fork 9
Auto-generate and persist TOKEN_SECRET on first run (#280) #281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
melvincarvalho
merged 4 commits into
JavaScriptSolidServer:gh-pages
from
melvincarvalho:issue-280-auto-persist-token-secret
Apr 21, 2026
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
bd6973c
Auto-generate and persist TOKEN_SECRET on first run (#280)
melvincarvalho b9801af
Address Copilot review on #281
melvincarvalho 28471d0
Address second Copilot round on #281
melvincarvalho bb13ae2
Doc/message accuracy (Copilot round 3 on #281)
melvincarvalho File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| /** | ||
| * TOKEN_SECRET resolution. | ||
| * | ||
| * Extracted from token.js so it can be unit-tested without pulling in the | ||
| * full auth graph (solid-oidc, nostr, webid-tls), which does module-level | ||
| * work that keeps the node:test event loop busy. | ||
| */ | ||
|
|
||
| import crypto from 'crypto'; | ||
| import fs from 'fs'; | ||
| import os from 'os'; | ||
| import path from 'path'; | ||
|
|
||
| export const DEFAULT_SECRET_PATH = path.join(os.homedir(), '.jss', 'token.secret'); | ||
|
|
||
| // Tighten permissions on POSIX, best-effort. No-op on Windows (ACLs) and | ||
| // on read-only filesystems — we never want perm-tightening to block using | ||
| // an otherwise-valid secret. | ||
| function chmodBestEffort(target, mode) { | ||
| try { | ||
| fs.chmodSync(target, mode); | ||
| } catch { | ||
| // Intentionally swallow — perms are defensive hardening, not required. | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Read a persisted secret from `filePath`, or generate one and write it | ||
| * (with dir mode 0700 and file mode 0600) if the file is missing. | ||
| * | ||
| * Read-first: if the file already exists and is non-empty we return it | ||
| * without trying to mkdir or tighten the containing directory. Deployments | ||
| * with a pre-provisioned secret on a read-only filesystem boot cleanly. | ||
| * | ||
| * Concurrent-startup safe: new secrets are written to a per-process temp | ||
| * file in the same directory and `renameSync`'d into place, so another | ||
| * process reading the target never sees a half-written file. If a peer | ||
| * process won the rename we fall back to reading their value. | ||
| * | ||
| * Anything other than ENOENT on the initial read (permission denied, | ||
| * corrupt FS, …) propagates. | ||
| */ | ||
| export function readOrWritePersistedSecret(filePath = DEFAULT_SECRET_PATH) { | ||
| const dir = path.dirname(filePath); | ||
|
|
||
| // Fast path: pre-existing non-empty file. We do not mkdir the parent | ||
| // dir here, and perm-tightening is best-effort (chmodBestEffort swallows | ||
| // all errors), so a pre-provisioned secret on a read-only filesystem | ||
| // still boots cleanly. | ||
| try { | ||
| const existing = fs.readFileSync(filePath, 'utf8').trim(); | ||
| if (existing) { | ||
| chmodBestEffort(dir, 0o700); | ||
| chmodBestEffort(filePath, 0o600); | ||
| return existing; | ||
| } | ||
| } catch (e) { | ||
| if (e.code !== 'ENOENT') throw e; | ||
| } | ||
|
|
||
| // Slow path: create it. Only touch the FS with writes from here on. | ||
| fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); | ||
| chmodBestEffort(dir, 0o700); | ||
|
|
||
| const generated = crypto.randomBytes(32).toString('hex'); | ||
| // Atomic write: fully write a temp file, then rename into place. On | ||
| // POSIX the rename is atomic, so concurrent readers see either the old | ||
| // content or the new complete content — never a half-written file. | ||
| const tmpPath = `${filePath}.${crypto.randomBytes(8).toString('hex')}.tmp`; | ||
| try { | ||
| fs.writeFileSync(tmpPath, generated, { mode: 0o600 }); | ||
| fs.renameSync(tmpPath, filePath); | ||
| } catch (e) { | ||
| try { fs.unlinkSync(tmpPath); } catch { /* ignore */ } | ||
| throw e; | ||
| } | ||
| chmodBestEffort(filePath, 0o600); | ||
|
|
||
| // Multiple processes racing each produce a different secret; only the | ||
| // last renamer's value sticks on disk. Re-read so every process ends up | ||
| // using the winning secret and token verification stays consistent. | ||
| const persisted = fs.readFileSync(filePath, 'utf8').trim(); | ||
| return persisted || generated; | ||
| } | ||
|
|
||
| /** | ||
| * Resolve the token secret. | ||
| * | ||
| * 1. TOKEN_SECRET env → use it. | ||
| * 2. Else read/create ~/.jss/token.secret. | ||
| * 3. On file-write failure: hard-exit in production, ephemeral secret otherwise. | ||
| * | ||
| * Console I/O is injected so tests can assert log behaviour without spamming | ||
| * the real console; defaults to the real console. | ||
| */ | ||
| export function resolveTokenSecret({ | ||
| env = process.env, | ||
| secretPath = DEFAULT_SECRET_PATH, | ||
| log = console, | ||
| exit = (code) => process.exit(code), | ||
| } = {}) { | ||
| if (env.TOKEN_SECRET) return env.TOKEN_SECRET; | ||
|
|
||
| try { | ||
| const s = readOrWritePersistedSecret(secretPath); | ||
| log.warn(`Using persisted TOKEN_SECRET at ${secretPath} (set TOKEN_SECRET env var to override).`); | ||
| return s; | ||
| } catch (e) { | ||
| if (env.NODE_ENV === 'production') { | ||
| const code = e?.code ? ` [${e.code}]` : ''; | ||
| log.error(`SECURITY ERROR: TOKEN_SECRET not set and ${secretPath} could not be read or created${code} (${e.message}).`); | ||
| log.error(`Set TOKEN_SECRET explicitly, or grant the necessary access to ${path.dirname(secretPath)}.`); | ||
| exit(1); | ||
| // `exit` is injectable; if a caller stubs it out we must not silently | ||
| // return undefined and let downstream code use an invalid secret. | ||
| throw new Error(`Failed to resolve TOKEN_SECRET in production: ${e.message}`); | ||
| } | ||
| const ephemeral = crypto.randomBytes(32).toString('hex'); | ||
| log.warn(`WARNING: Could not persist TOKEN_SECRET (${e.message}). Using ephemeral secret; tokens will not survive restarts.`); | ||
| return ephemeral; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| /** | ||
| * Unit tests for TOKEN_SECRET resolution (src/auth/token-secret.js). | ||
| * | ||
| * Covers #280: TOKEN_SECRET auto-persists on first run rather than hard-exiting. | ||
| */ | ||
|
|
||
| import { describe, it, before, after } from 'node:test'; | ||
| import assert from 'node:assert'; | ||
| import fs from 'fs'; | ||
| import os from 'os'; | ||
| import path from 'path'; | ||
| import { | ||
| readOrWritePersistedSecret, | ||
| resolveTokenSecret, | ||
| DEFAULT_SECRET_PATH, | ||
| } from '../src/auth/token-secret.js'; | ||
|
|
||
| describe('readOrWritePersistedSecret', () => { | ||
| let tmpDir; | ||
| let secretPath; | ||
|
|
||
| before(() => { | ||
| tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jss-token-secret-')); | ||
| secretPath = path.join(tmpDir, '.jss', 'token.secret'); | ||
| }); | ||
|
|
||
| after(() => { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| it('generates + persists a secret when the file is missing', () => { | ||
| const s = readOrWritePersistedSecret(secretPath); | ||
| assert.strictEqual(typeof s, 'string'); | ||
| assert.strictEqual(s.length, 64); // 32 bytes, hex-encoded | ||
| assert.strictEqual(fs.readFileSync(secretPath, 'utf8').trim(), s); | ||
| }); | ||
|
|
||
| it('returns the same secret on subsequent calls', () => { | ||
| const first = readOrWritePersistedSecret(secretPath); | ||
| const second = readOrWritePersistedSecret(secretPath); | ||
| assert.strictEqual(first, second); | ||
| }); | ||
|
|
||
| it('enforces tight permissions on POSIX (skipped on Windows)', { skip: process.platform === 'win32' }, () => { | ||
| const stat = fs.statSync(secretPath); | ||
| assert.strictEqual(stat.mode & 0o777, 0o600, 'secret file should be mode 0600'); | ||
| const dirStat = fs.statSync(path.dirname(secretPath)); | ||
| assert.strictEqual(dirStat.mode & 0o777, 0o700, 'secret dir should be mode 0700'); | ||
| }); | ||
|
|
||
| it('propagates errors other than ENOENT', () => { | ||
| // Use a regular file as the would-be parent directory — mkdirSync then | ||
| // fails with ENOTDIR synchronously. Portable across OSes. | ||
| const blockerFile = path.join(tmpDir, 'blocker-file'); | ||
| fs.writeFileSync(blockerFile, 'not a dir'); | ||
| const unwritable = path.join(blockerFile, '.jss', 'token.secret'); | ||
| assert.throws(() => readOrWritePersistedSecret(unwritable)); | ||
| }); | ||
|
|
||
| it('recovers when the secret file already exists but is empty', () => { | ||
| // Simulates a concurrent or interrupted persistence case: the file | ||
| // is present (so the fast path falls through the trim-empty check) | ||
| // but carries no usable secret yet. tmp-file + renameSync repairs | ||
| // it by overwriting atomically. | ||
| const p = path.join(tmpDir, 'empty', '.jss', 'token.secret'); | ||
| fs.mkdirSync(path.dirname(p), { recursive: true }); | ||
| fs.writeFileSync(p, ''); | ||
| const s = readOrWritePersistedSecret(p); | ||
| assert.strictEqual(s.length, 64); | ||
| assert.strictEqual(fs.readFileSync(p, 'utf8').trim(), s); | ||
| }); | ||
|
|
||
| it('tightens permissions when the file already exists with loose mode', { skip: process.platform === 'win32' }, () => { | ||
| const p = path.join(tmpDir, 'loose', '.jss', 'token.secret'); | ||
| fs.mkdirSync(path.dirname(p), { recursive: true, mode: 0o755 }); | ||
| fs.writeFileSync(p, 'a'.repeat(64), { mode: 0o644 }); | ||
| readOrWritePersistedSecret(p); | ||
| assert.strictEqual(fs.statSync(p).mode & 0o777, 0o600); | ||
| assert.strictEqual(fs.statSync(path.dirname(p)).mode & 0o777, 0o700); | ||
| }); | ||
|
|
||
| it('reads a pre-existing secret even when the parent dir is not writable', { skip: process.platform === 'win32' || process.getuid?.() === 0 }, () => { | ||
| // Simulates a read-only deployment: secret provisioned ahead of time, | ||
| // parent dir not writable for the current user. Must not block startup. | ||
| const p = path.join(tmpDir, 'readonly-parent', '.jss', 'token.secret'); | ||
| fs.mkdirSync(path.dirname(p), { recursive: true, mode: 0o700 }); | ||
| const expected = 'b'.repeat(64); | ||
| fs.writeFileSync(p, expected); | ||
| fs.chmodSync(path.dirname(p), 0o500); // r-x, no write | ||
| try { | ||
| const s = readOrWritePersistedSecret(p); | ||
| assert.strictEqual(s, expected); | ||
| } finally { | ||
| fs.chmodSync(path.dirname(p), 0o700); // let after()'s rmSync clean up | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('resolveTokenSecret', () => { | ||
| let tmpDir; | ||
|
|
||
| before(() => { | ||
| tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jss-resolve-secret-')); | ||
| }); | ||
|
|
||
| after(() => { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| const silentLog = { warn: () => {}, error: () => {} }; | ||
|
|
||
| it('prefers TOKEN_SECRET env var', () => { | ||
| const s = resolveTokenSecret({ | ||
| env: { TOKEN_SECRET: 'from-env' }, | ||
| secretPath: path.join(tmpDir, 'unused', 'token.secret'), | ||
| log: silentLog, | ||
| }); | ||
| assert.strictEqual(s, 'from-env'); | ||
| }); | ||
|
|
||
| it('persists a generated secret when env is unset', () => { | ||
| const p = path.join(tmpDir, 'persist', 'token.secret'); | ||
| const s = resolveTokenSecret({ env: {}, secretPath: p, log: silentLog }); | ||
| assert.strictEqual(s.length, 64); | ||
| assert.strictEqual(fs.readFileSync(p, 'utf8').trim(), s); | ||
| }); | ||
|
|
||
| it('returns the same persisted secret on the next call', () => { | ||
| const p = path.join(tmpDir, 'persist-twice', 'token.secret'); | ||
| const first = resolveTokenSecret({ env: {}, secretPath: p, log: silentLog }); | ||
| const second = resolveTokenSecret({ env: {}, secretPath: p, log: silentLog }); | ||
| assert.strictEqual(first, second); | ||
| }); | ||
|
|
||
| // Build an unwritable path by planting a regular file where the helper | ||
| // would try to mkdir a directory. mkdirSync then fails synchronously. | ||
| function buildUnwritable(name) { | ||
| const blocker = path.join(tmpDir, name, 'blocker-file'); | ||
| fs.mkdirSync(path.dirname(blocker), { recursive: true }); | ||
| fs.writeFileSync(blocker, 'not a dir'); | ||
| return path.join(blocker, '.jss', 'token.secret'); | ||
| } | ||
|
|
||
| it('hard-exits in production when persistence fails', () => { | ||
| let exitCode; | ||
| assert.throws(() => { | ||
| resolveTokenSecret({ | ||
| env: { NODE_ENV: 'production' }, | ||
| secretPath: buildUnwritable('prod'), | ||
| log: silentLog, | ||
| exit: (code) => { exitCode = code; }, // stubbed — doesn't actually terminate | ||
| }); | ||
| }); | ||
| // exit(1) must still have been invoked even though we throw afterwards, | ||
| // so a non-stubbed production process actually terminates. | ||
| assert.strictEqual(exitCode, 1); | ||
| }); | ||
|
|
||
| it('throws after exit so a stubbed exit() cannot leak undefined downstream', () => { | ||
| // Regression: earlier versions returned undefined "for tests" after | ||
| // calling exit(), which could let callers continue with an invalid | ||
| // secret when exit is stubbed. | ||
| assert.throws( | ||
| () => resolveTokenSecret({ | ||
| env: { NODE_ENV: 'production' }, | ||
| secretPath: buildUnwritable('no-leak'), | ||
| log: silentLog, | ||
| exit: () => {}, | ||
| }), | ||
| /TOKEN_SECRET/ | ||
| ); | ||
| }); | ||
|
|
||
| it('production error message references the actual secret directory', () => { | ||
| const secretPath = buildUnwritable('custom-path'); | ||
| const errors = []; | ||
| assert.throws(() => { | ||
| resolveTokenSecret({ | ||
| env: { NODE_ENV: 'production' }, | ||
| secretPath, | ||
| log: { warn: () => {}, error: (msg) => errors.push(msg) }, | ||
| exit: () => {}, | ||
| }); | ||
| }); | ||
| assert.ok( | ||
| errors.some(m => m.includes(path.dirname(secretPath))), | ||
| `expected an error to mention ${path.dirname(secretPath)}, got: ${errors.join(' | ')}` | ||
| ); | ||
| }); | ||
|
|
||
| it('falls back to an ephemeral secret outside production when persistence fails', () => { | ||
| const s = resolveTokenSecret({ | ||
| env: {}, | ||
| secretPath: buildUnwritable('dev'), | ||
| log: silentLog, | ||
| exit: () => { throw new Error('exit should not be called in dev') }, | ||
| }); | ||
| assert.strictEqual(typeof s, 'string'); | ||
| assert.strictEqual(s.length, 64); | ||
| }); | ||
| }); | ||
|
|
||
| describe('DEFAULT_SECRET_PATH', () => { | ||
| it('is absolute and platform-native', () => { | ||
| assert.ok(path.isAbsolute(DEFAULT_SECRET_PATH)); | ||
| assert.ok(DEFAULT_SECRET_PATH.includes('.jss')); | ||
| assert.ok(DEFAULT_SECRET_PATH.includes('token.secret')); | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The “safe against multiple processes starting in parallel” claim isn’t fully upheld:
writeFileSync(..., { flag: 'wx' })creates the file before it is fully written, so another process that hitsEEXISTcan read an empty/partial file and then overwrite it in the fallback path, leading to different secrets across processes. To make convergence robust, consider an atomic write strategy (e.g., write to a temp file in the same directory andrenameSyncinto place, or retry reads until non-empty instead of overwriting).