refactor(persistence): migrate share-store to canonical atomic-write util (DC-102) [glm-grade=A]
Drops share-store's private _atomicWriteJSON copy (pid+Date.now() tmp names, no fsync, no failure cleanup) in favor of src/utils/atomic-write.js: fsync'd same-dir tmp+rename, exclusive-create 0600, parent-dir fsync, cleanup-on-failure. Also fixes a latent bug in the same file: .share-secret signing-key persistence used plain fs.writeFileSync — a torn write would silently rotate the HMAC key on next boot, invalidating every outstanding share signature (links 404, no error logged). Now atomicWriteFile. Sole consumers parse JSON / trim(), so the dropped trailing newline on shares.json is unobservable. +2 regression tests pin 0600 on both files, complete content, zero tmp leftovers, and HMAC-still-verifies via getRaw. Judge: GLM-5.3 cold read, round-1 A, deleg_50d5c239 (41s, 4 calls). Verdict: urn:ump:ehblegyr5eko4hgyfylnrqk72hh3crmc43o2yt5ragfgvyc4zrdq Full suite: 121 suites / 2774 tests green.
This commit is contained in:
@@ -297,3 +297,66 @@ describe('DC-101 user-store on canonical atomic-write (real fs)', () => {
|
|||||||
expect(leftovers).toEqual([]);
|
expect(leftovers).toEqual([]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// DC-102: share-store migrated off its private _atomicWriteJSON copy onto
|
||||||
|
// the canonical writer. Store-level pins: shares.json AND the .share-secret
|
||||||
|
// signing key land as complete content at mode 0600 with no temp leftovers —
|
||||||
|
// a torn secret write would silently rotate the key and invalidate every
|
||||||
|
// outstanding share signature on next boot.
|
||||||
|
describe('DC-102 share-store on canonical atomic-write (real fs)', () => {
|
||||||
|
let dir, store;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dc102-share-'));
|
||||||
|
store = require('../src/security/share-store').createShareStore({ dataDir: dir });
|
||||||
|
});
|
||||||
|
afterEach(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch (_) {} });
|
||||||
|
|
||||||
|
test('issued share + persisted signing secret land at 0600, complete, no temp leftovers', async () => {
|
||||||
|
const r = await store.issuePublic({ serviceId: 'svc', ttlMs: 60 * 60 * 1000 });
|
||||||
|
expect(r.ok).toBe(true);
|
||||||
|
|
||||||
|
const sharesFile = path.join(dir, 'shares.json');
|
||||||
|
const secretFile = path.join(dir, '.share-secret');
|
||||||
|
const sharesSt = fs.statSync(sharesFile);
|
||||||
|
const secretSt = fs.statSync(secretFile);
|
||||||
|
expect(sharesSt.mode & 0o777).toBe(0o600);
|
||||||
|
expect(secretSt.mode & 0o777).toBe(0o600);
|
||||||
|
|
||||||
|
// complete JSON — a torn write would fail JSON.parse right here
|
||||||
|
const data = JSON.parse(fs.readFileSync(sharesFile, 'utf8'));
|
||||||
|
expect(Object.keys(data.shares)).toHaveLength(1);
|
||||||
|
// complete secret — readable, 32+ bytes after trim, trailing newline kept
|
||||||
|
const secret = fs.readFileSync(secretFile, 'utf8');
|
||||||
|
expect(secret.trim().length).toBeGreaterThanOrEqual(32);
|
||||||
|
expect(secret.endsWith('\n')).toBe(true);
|
||||||
|
|
||||||
|
const leftovers = fs.readdirSync(dir).filter((f) => f !== 'shares.json' && f !== '.share-secret');
|
||||||
|
expect(leftovers).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('back-to-back mutations (issue x2, subscribe, tailscale use, revoke) never collide on tmp names', async () => {
|
||||||
|
const a = await store.issuePublic({ serviceId: 'svc', subscribeCap: 5 });
|
||||||
|
const b = await store.issueTailscale({ serviceId: 'svc', email: 'dc102@x.com' });
|
||||||
|
await store.recordPublicSubscribe(a.token, { email: 'sub@x.com' });
|
||||||
|
await store.recordTailscaleUse(b.token, { deviceId: 'device-1' });
|
||||||
|
await store.revoke(a.id);
|
||||||
|
|
||||||
|
// b remains outstanding and fully redeemable state on disk
|
||||||
|
const data = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
|
||||||
|
expect(Object.keys(data.shares)).toEqual([b.id]);
|
||||||
|
expect(data.shares[b.id].usedAt).toBeTruthy();
|
||||||
|
expect(data.shares[b.id].usedBy).toBe('device-1');
|
||||||
|
|
||||||
|
// signature verification still passes against the atomically persisted
|
||||||
|
// secret — getRaw checks hash + HMAC only (not used-state), so a rotated
|
||||||
|
// or torn secret would return null here.
|
||||||
|
const raw = await store.getRaw(b.token);
|
||||||
|
expect(raw).toBeTruthy();
|
||||||
|
expect(raw.id).toBe(b.id);
|
||||||
|
expect(raw.kind).toBe('tailscale');
|
||||||
|
|
||||||
|
const leftovers = fs.readdirSync(dir).filter((f) => f !== 'shares.json' && f !== '.share-secret');
|
||||||
|
expect(leftovers).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -12,9 +12,10 @@
|
|||||||
* to a device tag. Invitee clicks the link → device joins the tailnet →
|
* to a device tag. Invitee clicks the link → device joins the tailnet →
|
||||||
* Caddy forward_auth inducts them into the service. Single-use, 24h TTL.
|
* Caddy forward_auth inducts them into the service. Single-use, 24h TTL.
|
||||||
*
|
*
|
||||||
* Storage: data/shares.json. Atomic writes via tmp+rename. The on-disk shape
|
* Storage: data/shares.json. Atomic durable writes via the canonical
|
||||||
* is identical to the invite store — UUID-keyed map of records with SHA-256
|
* shared atomic-write util (DC-099/DC-102) — fsync'd tmp+rename. The on-disk
|
||||||
* hashed tokens. Raw token is only returned at issue() time.
|
* shape is identical to the invite store — UUID-keyed map of records with
|
||||||
|
* SHA-256 hashed tokens. Raw token is only returned at issue() time.
|
||||||
*
|
*
|
||||||
* Public-share token also carries a HMAC signature binding it to the
|
* Public-share token also carries a HMAC signature binding it to the
|
||||||
* serviceId so a leaked token cannot be silently retargeted. The signature
|
* serviceId so a leaked token cannot be silently retargeted. The signature
|
||||||
@@ -37,6 +38,7 @@ const path = require('path');
|
|||||||
const fs = require('fs');
|
const fs = require('fs');
|
||||||
const crypto = require('crypto');
|
const crypto = require('crypto');
|
||||||
const platformPaths = require('../../platform-paths');
|
const platformPaths = require('../../platform-paths');
|
||||||
|
const { atomicWriteFile, atomicWriteJSON } = require('../utils/atomic-write');
|
||||||
|
|
||||||
const DEFAULT_PUBLIC_TTL_MS = 24 * 60 * 60 * 1000; // 24h
|
const DEFAULT_PUBLIC_TTL_MS = 24 * 60 * 60 * 1000; // 24h
|
||||||
const DEFAULT_TAILSCALE_TTL_MS = 24 * 60 * 60 * 1000; // 24h
|
const DEFAULT_TAILSCALE_TTL_MS = 24 * 60 * 60 * 1000; // 24h
|
||||||
@@ -97,12 +99,6 @@ function validatePublicDeviceId(raw) {
|
|||||||
function _nowMs() { return Date.now(); }
|
function _nowMs() { return Date.now(); }
|
||||||
function _nowIso() { return new Date().toISOString(); }
|
function _nowIso() { return new Date().toISOString(); }
|
||||||
|
|
||||||
function _atomicWriteJSON(filePath, data) {
|
|
||||||
const tmp = filePath + '.tmp.' + process.pid + '.' + Date.now();
|
|
||||||
fs.writeFileSync(tmp, JSON.stringify(data, null, 2) + '\n', { mode: 0o600 });
|
|
||||||
fs.renameSync(tmp, filePath);
|
|
||||||
}
|
|
||||||
|
|
||||||
function _readJSON(filePath, fallback) {
|
function _readJSON(filePath, fallback) {
|
||||||
try {
|
try {
|
||||||
const raw = fs.readFileSync(filePath, 'utf8');
|
const raw = fs.readFileSync(filePath, 'utf8');
|
||||||
@@ -150,7 +146,12 @@ function createShareStore(opts = {}) {
|
|||||||
} catch (_) { /* missing or unreadable — generate fresh */ }
|
} catch (_) { /* missing or unreadable — generate fresh */ }
|
||||||
const fresh = crypto.randomBytes(32).toString('base64url');
|
const fresh = crypto.randomBytes(32).toString('base64url');
|
||||||
try {
|
try {
|
||||||
fs.writeFileSync(_secretFile, fresh + '\n', { mode: 0o600 });
|
// DC-102: canonical atomic write. A torn `.share-secret` write would
|
||||||
|
// silently rotate the signing key on next boot — invalidating every
|
||||||
|
// outstanding share signature (all peeks fail, links 404) — with no
|
||||||
|
// error anywhere. fsync'd tmp+rename guarantees the file is either the
|
||||||
|
// complete old secret or the complete new one.
|
||||||
|
atomicWriteFile(_secretFile, fresh + '\n');
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
log.warn && log.warn('share', 'failed to persist signing secret', { err: err && err.message });
|
log.warn && log.warn('share', 'failed to persist signing secret', { err: err && err.message });
|
||||||
}
|
}
|
||||||
@@ -170,7 +171,7 @@ function createShareStore(opts = {}) {
|
|||||||
if (!data.shares || typeof data.shares !== 'object') data.shares = {};
|
if (!data.shares || typeof data.shares !== 'object') data.shares = {};
|
||||||
return data;
|
return data;
|
||||||
}
|
}
|
||||||
function _save(data) { _atomicWriteJSON(file, data); }
|
function _save(data) { atomicWriteJSON(file, data); }
|
||||||
|
|
||||||
function _prune(data) {
|
function _prune(data) {
|
||||||
const cutoff = _nowMs() - PRUNE_AFTER_MS;
|
const cutoff = _nowMs() - PRUNE_AFTER_MS;
|
||||||
|
|||||||
Reference in New Issue
Block a user