Skip to content

Commit 2de873e

Browse files
melvincarvalhomelvincarvalho
committed
review: address Copilot pickups on JavaScriptSolidServer#442
Six small but real fixes from the first review pass. 1. storage.write created the file with the process umask (often 0644) and only then chmod'd to the requested mode. Race window where another local process could read freshly created secret material before chmod ran. Pass `mode` to fs.writeFile at create time so the file is *born* with the right perms; keep the chmod afterward as belt-and-braces for the overwrite case (Node only honours mode at create time, not on overwrite). 2. createRootPodStructure's docstring said the secret was "never returned", but the returned ownerKey object includes secretHex and secretKeyMultibase. Tightened the doc to match reality — "callers must not log the secret" — and noted why we keep the internal representation full (tests, future one-shot signing). 3. Single-user onReady built the same `keyPath` string in both branches of an isRootPod ternary. Collapsed to one expression (podUri already carries the trailing slash + name segment). 4. docs/provision-keys.md intro referenced "JavaScriptSolidServer#427 / JavaScriptSolidServer#437" — but JavaScriptSolidServer#427 is the unrelated ACL-portability umbrella. Just JavaScriptSolidServer#437. 5. docs backup example only showed the root-pod path. Added the named-pod variants for both the HTTP curl backup and the on-disk cp backup, since the named-pod layout is the multi-user case. 6. Updated storage.write JSDoc to document the new `options.mode` argument with the rationale. 840/840 tests pass.
1 parent 79f128c commit 2de873e

3 files changed

Lines changed: 59 additions & 23 deletions

File tree

‎docs/provision-keys.md‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Owner-key provisioning (`--provision-keys`)
22

3-
Phase 1 of [#427 / #437](https://github.com/JavaScriptSolidServer/JavaScriptSolidServer/issues/437). Generates a Schnorr secp256k1 keypair on pod creation and writes it as a W3C [Controlled Identifiers v1.0 Multikey](https://www.w3.org/TR/cid-1.0/) document at `<pod>/private/privkey.jsonld`.
3+
Phase 1 of [#437](https://github.com/JavaScriptSolidServer/JavaScriptSolidServer/issues/437). Generates a Schnorr secp256k1 keypair on pod creation and writes it as a W3C [Controlled Identifiers v1.0 Multikey](https://www.w3.org/TR/cid-1.0/) document at `<pod>/private/privkey.jsonld`.
44

55
The same key is intended to serve, over time, as: a Solid signing identity, a Nostr identity (same curve), and a `did:nostr:` DID controller (Phase 2). One CLI flag → pod-resident self-sovereign identity ready for both Solid and Nostr / agentic use cases.
66

@@ -113,14 +113,23 @@ Each upgrade is a wrapper around the same secret material. You're not picking a
113113
This is not optional.
114114

115115
```bash
116-
# Authenticate as owner via HTTP (preferred — works even on a remote host)
116+
# Authenticate as owner via HTTP (preferred — works even on a remote host).
117+
# For a single-user / root pod the key is at <pod>/private/privkey.jsonld;
118+
# for a named pod (multi-user) it's at <pod>/<name>/private/privkey.jsonld.
117119
curl -H "Authorization: Bearer <owner-token>" \
118120
http://your.example/private/privkey.jsonld \
119121
-o pod-key-backup.jsonld
122+
# named-pod variant:
123+
curl -H "Authorization: Bearer <owner-token>" \
124+
http://your.example/alice/private/privkey.jsonld \
125+
-o pod-key-backup.jsonld
120126
chmod 600 pod-key-backup.jsonld
121127

122-
# Or copy from disk if you have local access (preserves perms)
123-
cp -p /path/to/data/private/privkey.jsonld pod-key-backup.jsonld
128+
# Or copy from disk if you have local access (preserves perms).
129+
# Root pod (single-user, default since #348):
130+
cp -p <DATA_ROOT>/private/privkey.jsonld pod-key-backup.jsonld
131+
# Named pod (multi-user, or single-user with --single-user-name=alice):
132+
cp -p <DATA_ROOT>/alice/private/privkey.jsonld pod-key-backup.jsonld
124133
```
125134

126135
Store the backup somewhere that survives the pod's host: another machine, encrypted cloud storage, a hardware token, a sealed envelope in a safe — whatever you'd use for an SSH key you actually care about. Losing this file means losing this identity permanently. There is no recovery flow in Phase 1.

‎src/server.js‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -861,11 +861,12 @@ export function createServer(options = {}) {
861861
// Surface the public side of any provisioned owner key, plus
862862
// a prominent backup reminder. The secret is NOT logged — it
863863
// lives on disk only, under /private/privkey.jsonld with
864-
// owner-only WAC and file mode 0600.
864+
// owner-only WAC and file mode 0o600.
865865
if (creation?.ownerKey) {
866-
const keyPath = isRootPod
867-
? `${podUri}private/privkey.jsonld`
868-
: `${podUri}private/privkey.jsonld`;
866+
// `podUri` already includes the trailing slash + any pod
867+
// name segment, so the same expression covers root and
868+
// named single-user pods.
869+
const keyPath = `${podUri}private/privkey.jsonld`;
869870
fastify.log.info(`Provisioned Schnorr secp256k1 owner key`);
870871
fastify.log.info(` Public key file: ${keyPath}`);
871872
fastify.log.info(` publicKeyMultibase: ${creation.ownerKey.publicMultibase}`);
@@ -1008,9 +1009,13 @@ export function createServer(options = {}) {
10081009

10091010
/**
10101011
* Create root-level pod structure (for single-user mode with pod at /).
1011-
* Returns `{ ownerKey }` when --provision-keys is set so the caller can
1012-
* surface the public side in the startup banner. The secret is never
1013-
* returned (it's only on disk under /private/, mode 0600).
1012+
* When --provision-keys is set, returns `{ ownerKey }` so the caller
1013+
* can surface the public side in the startup banner. The returned
1014+
* `ownerKey` includes secretHex and secretKeyMultibase — needed by
1015+
* tests, present in case a future caller needs to perform a one-shot
1016+
* sign before the file is read back via WAC. **Callers must not log
1017+
* the secret.** The secret's only durable home is the on-disk file
1018+
* under /private/ (mode 0o600, owner-only WAC).
10141019
*/
10151020
async function createRootPodStructure(webId, podUri, issuer, displayName) {
10161021
const { generateProfile, generatePreferences, generateTypeIndex, serialize } = await import('./webid/profile.js');

‎src/storage/filesystem.js‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,29 +74,51 @@ export function createReadStream(urlPath, options = {}) {
7474
}
7575

7676
/**
77-
* Write resource content
78-
* @param {string} urlPath
79-
* @param {Buffer | string} content
80-
* @returns {Promise<boolean>}
77+
* Write resource content.
78+
*
79+
* @param {string} urlPath - URL path of the resource being written
80+
* (translated to a filesystem path internally).
81+
* @param {Buffer | string} content - Bytes / text to write. Replaces
82+
* the file if it already exists.
83+
* @param {object} [options]
84+
* @param {number} [options.mode] - POSIX file mode (e.g. `0o600`) to
85+
* apply to the created file. Passed to `fs.writeFile` at create
86+
* time so the file is never visible to other unix users with a
87+
* looser default; an additional `chmod` runs afterward to tighten
88+
* the file when overwriting an existing path that was created with
89+
* a wider mode. No-op on Windows. See #437 for the secret-material
90+
* use case.
91+
* @returns {Promise<boolean>} `true` on success, `false` on write
92+
* failure (chmod failures are logged but do not fail the write).
8193
*/
8294
export async function write(urlPath, content, options = {}) {
8395
const filePath = urlToPath(urlPath);
8496

8597
try {
8698
// Ensure parent directory exists
8799
await fs.ensureDir(path.dirname(filePath));
88-
await fs.writeFile(filePath, content);
89-
// Optional file-mode tightening (e.g. 0o600 for secret material).
90-
// No-op on Windows, where chmod permissions are coarse — callers
91-
// should not rely on POSIX modes for cross-platform security.
100+
101+
// Pass `mode` to writeFile so the file is *created* with the
102+
// requested permissions, closing the race window where another
103+
// local process could read a freshly created secret-material
104+
// file before a follow-up `chmod` ran. (Node only honours `mode`
105+
// at create time, never on overwrite.)
106+
if (typeof options.mode === 'number') {
107+
await fs.writeFile(filePath, content, { mode: options.mode });
108+
} else {
109+
await fs.writeFile(filePath, content);
110+
}
111+
112+
// Belt-and-braces: when overwriting an existing file, writeFile
113+
// does NOT change the existing mode — apply chmod so a stale 0644
114+
// file gets tightened to 0600 on subsequent writes. Logged but
115+
// non-fatal: callers that care about strict permissions should
116+
// also rely on filesystem-level protection (FDE / OS keyring /
117+
// container user namespacing).
92118
if (typeof options.mode === 'number') {
93119
try {
94120
await fs.chmod(filePath, options.mode);
95121
} catch (chmodErr) {
96-
// Don't fail the write because the chmod didn't take — log and
97-
// continue. Callers that care about strict permissions (secret
98-
// material) should additionally rely on filesystem-level
99-
// protection (FDE, OS keyring, container user namespacing).
100122
console.warn(`chmod ${options.mode.toString(8)} on ${filePath} failed:`, chmodErr.message);
101123
}
102124
}

0 commit comments

Comments
 (0)