Skip to content

feat: Phase 2 — wire owner key into WebID profile + did:nostr controller (#443)#444

Merged
melvincarvalho merged 6 commits into
gh-pagesfrom
issue-443-phase-2-wire-key-into-profile
May 14, 2026
Merged

feat: Phase 2 — wire owner key into WebID profile + did:nostr controller (#443)#444
melvincarvalho merged 6 commits into
gh-pagesfrom
issue-443-phase-2-wire-key-into-profile

Conversation

@melvincarvalho

Copy link
Copy Markdown
Contributor

Summary

Closes the LWS-CID auth loop with the Phase 1 keypair. Three small wires:

  1. Inject the Phase 1 public key into the seeded WebID profile. provisionOwnerKey() now also returns a verificationMethod entry (Multikey + JWK forms). createPodStructure / createRootPodStructure pass it to generateProfile so the existing LWS-CID verifier in src/auth/lws-cid.js can authenticate JWTs signed with the on-disk secret. Same VM id is referenced from authentication and assertionMethod so the key counts as an auth factor without an app having to PATCH those arrays.
  2. Flip the Multikey controller in /private/privkey.jsonld from the WebID to did:nostr:<hex> (resolves locally via the existing resolveDidNostrLocally / resolveDidNostrToWebId). Phase 1's design log explicitly anticipated this swap; Phase 2 ships it.
  3. Normalize the secret to BIP-340 even-y form at generation time. Without this, ECDSA sigs from the raw secret would verify against the natural y of G*secret (even/odd ~50/50), while the JWK we publish is derived from the even-y x-only Schnorr pubkey — Phase 2's round-trip would flake non-deterministically. Normalizing once means one secret works under both Schnorr (Nostr) and ECDSA (LWS-CID JWT) with no parity gymnastics.

No new dependencies. No changes to src/auth/lws-cid.js, src/auth/did-nostr.js, or the profile generator's @context. Two wires + controller flip + parity fix.

Tests

npm test854/854 passing.

  • Unit (test/keys-provision.test.js): did:nostr controller default + explicit override (Phase 1 backward-compat), VM shape (Multikey + JWK), legacy controllerWebId arg still honoured, secret normalization stress (64 iterations).
  • Integration (test/keys-provision-integration.test.js): direct createPodStructure call returns the new ownerKey shape (vm + didNostr).
  • End-to-end round-trip (new file test/keys-provision-lws-cid.test.js): provision a key → generate a profile with the VM → sign an LWS-CID JWT with the on-disk secret → call verifyLwsCidAuth → returns the pod owner WebID. Plus a negative case (mismatched secret rejects with signature-verification error). This is the test that proves Phase 2 closes the loop.

What's already in tree (didn't need building)

Capability Where
LWS-CID JWT verifier src/auth/lws-cid.js
did:nostr: resolver + local well-known DID-doc src/auth/did-nostr.js, src/idp/well-known-did-nostr.js
WebID profile generator with CID v1.0 verificationMethod slot src/webid/profile.js
Pubkey extractor from a VM (handles Multikey + JWK with BIP-340 even-y check) src/auth/nostr-keys.js
Phase 1 keypair generator + Multikey serializer src/keys/provision.js

Test plan

  • npm test — 854/854 locally
  • End-to-end LWS-CID round-trip (positive + negative) covered by a dedicated test file
  • Backward-compat: legacy controllerWebId arg + explicit controller override still honoured
  • Parity-normalization stress (64 iterations all even-y)

Out of scope (Phase 3+)

  • --key-passphrase / scrypt+aesgcm wrap (Phase 3)
  • BIP-39 seed phrase derivation (Phase 3)
  • Hardware wallets / KMS / encrypted-at-rest (Phase 4)

Closes #443. Phase 2 of #437.

…ler (#443)

Closes the LWS-CID auth loop with the Phase 1 keypair.

Three wires:

1. provisionOwnerKey() now also produces a verificationMethod entry
   (publicKeyMultibase for CID v1.0 conformance + publicKeyJwk for
   the LWS-CID verifier). createPodStructure / createRootPodStructure
   inject the VM into the seeded WebID profile so the existing
   verifier in src/auth/lws-cid.js can authenticate JWTs signed with
   the on-disk secret. The same VM `id` is referenced from
   `authentication` and `assertionMethod` so the key counts as an
   auth factor without an app having to PATCH those arrays.

2. The Multikey document at /private/privkey.jsonld now uses
   `did:nostr:<hex>` as its controller (was the WebID in Phase 1).
   The Phase 1 design log explicitly anticipated this swap once the
   resolver landed; jss has had `resolveDidNostrLocally` and
   `resolveDidNostrToWebId` for a while, so the controller is no
   longer a dangling reference. Backward-compat: callers can still
   pass an explicit `controller` if they want the WebID form.

3. generateOwnerKeypair now normalizes the secret so G*secret has
   even y (BIP-340 convention). Without normalization, ECDSA
   signatures made with the raw secret would verify against the
   *natural* y of the public point — even/odd ~50/50 — while the
   JWK we publish in the profile is derived from the even-y x-only
   Schnorr pubkey. Phase 2's LWS-CID round-trip would flake non-
   deterministically. Normalizing once at generation means a single
   secret-on-disk works under both Schnorr (Nostr) and ECDSA
   (LWS-CID JWT) without parity gymnastics in either path.

Tests:

- 6 new unit tests in test/keys-provision.test.js: did:nostr
  controller default, explicit override, VM shape (Multikey + JWK),
  legacy controllerWebId arg backward-compat, secret normalization
  stress (64 iterations).
- 1 new integration test asserting the direct createPodStructure
  call returns the new ownerKey shape (vm + didNostr).
- 1 new test file (test/keys-provision-lws-cid.test.js) with the
  end-to-end round-trip: provision a key, generate a profile with
  the VM, sign an LWS-CID JWT with the on-disk secret, verify via
  the existing verifyLwsCidAuth — returns the WebID. Plus a
  negative test: signing with a different secret rejects with a
  signature-verification error.

No new dependencies. No changes to the LWS-CID verifier, the
did:nostr resolver, or the profile generator's @context. Phase 2
is two wires plus a controller flip plus a parity-normalization
fix to make the wires actually carry signal.

Closes #443. Phase 2 of #437.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Phase 2 of --provision-keys (#437/#443): closes the LWS-CID auth loop by injecting the Phase-1 owner key's public side into the seeded WebID profile and flipping the on-disk Multikey document's controller from the WebID to did:nostr:<hex>. Also normalizes the freshly generated secret to the BIP-340 even-y form so a single secret-on-disk works under both Schnorr and ECDSA verification.

Changes:

  • provisionOwnerKey now returns a vm (Multikey + JWK) and didNostr; default Multikey-document controller is did:nostr:<hex> (legacy controllerWebId/controller arguments still honoured).
  • Pod creation (src/handlers/container.js, src/server.js) generates the keypair up-front and seeds the profile's verificationMethod / authentication / assertionMethod from it.
  • generateOwnerKeypair negates the random secret when G*secret has odd y, ensuring deterministic agreement with the published x-only/JWK pubkey.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/keys/provision.js Even-y normalization, did:nostr controller default, new buildOwnerVerificationMethod, publicKeyJwkFromHex, didNostrFromPublicHex, expanded provisionOwnerKey return shape with backward-compat parameter handling.
src/webid/profile.js generateProfileJsonLd / generateProfile accept an optional ownerVm and lands it in verificationMethod + authentication + assertionMethod.
src/handlers/container.js createPodStructure provisions the keypair before profile generation, injects VM, persists privkey last.
src/server.js createRootPodStructure mirrors the same reordering for single-user / root pods.
test/keys-provision.test.js Updated unit tests for new controller default, JWK/VM shape, legacy controllerWebId arg, parity normalization stress (64 iters).
test/keys-provision-integration.test.js Asserts on-disk controller is did:nostr:<hex> and result.ownerKey carries vm + didNostr.
test/keys-provision-lws-cid.test.js New end-to-end LWS-CID round-trip (positive + negative) using stubbed global.fetch to serve the generated profile.
Comments suppressed due to low confidence (1)

src/keys/provision.js:249

  • The thrown error provisionOwnerKey: webId required is misleading for callers that still use the legacy controllerWebId argument (which this function explicitly continues to accept on the same line). A user passing provisionOwnerKey({ controllerWebId: '' }) will see "webId required" even though they never tried to pass webId. Consider mentioning both accepted argument names, or — preferably — settling on a single canonical name and updating the error message accordingly.
  const effectiveWebId = webId ?? controllerWebId;
  if (typeof effectiveWebId !== 'string' || !effectiveWebId) {
    throw new Error('provisionOwnerKey: webId required');
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/keys/provision.js Outdated
Comment on lines 242 to 264
export function provisionOwnerKey({ webId, controller, controllerWebId }) {
// Backward-compat: callers from Phase 1 passed `controllerWebId` and
// expected it to land in both the document's `controller` field and
// (implicitly) the VM controller. Map it to the new shape.
const effectiveWebId = webId ?? controllerWebId;
if (typeof effectiveWebId !== 'string' || !effectiveWebId) {
throw new Error('provisionOwnerKey: webId required');
}
const { publicHex, secretHex } = generateOwnerKeypair();
const document = buildOwnerKeyDocument({ controllerWebId, publicHex, secretHex });
const document = buildOwnerKeyDocument({
publicHex,
secretHex,
controller: controller ?? (controllerWebId ?? didNostrFromPublicHex(publicHex))
});
const vm = buildOwnerVerificationMethod({ webId: effectiveWebId, publicHex });
return {
document,
vm,
publicHex,
secretHex,
publicMultibase: document.publicKeyMultibase
publicMultibase: document.publicKeyMultibase,
didNostr: didNostrFromPublicHex(publicHex)
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 2285d05: dropped the controllerWebId legacy alias and renamed the doc-override knob to documentController. Two unambiguous inputs now (webId for VM controller + VM @id, documentController to override the Multikey doc's controller, defaulting to did:nostr:). All in-tree callers migrated — no external break.

Comment thread src/handlers/container.js
Comment on lines +202 to 285
const ownerKey = options.provisionKeys === true
? provisionOwnerKey({ webId })
: null;

// Generate and write WebID profile at /profile/card.jsonld. When
// an owner key was provisioned, its VM lands in the profile so the
// existing LWS-CID verifier (src/auth/lws-cid.js) can authenticate
// JWTs signed with the matching secret.
const profile = generateProfile({ webId, name, podUri, issuer, ownerVm: ownerKey?.vm });
await storage.write(`${podPath}profile/card.jsonld`, serialize(profile));

// Generate and write preferences
const prefs = generatePreferences({ webId, podUri });
await storage.write(`${podPath}settings/prefs.jsonld`, serialize(prefs));

// Generate and write type indexes
const publicTypeIndex = generateTypeIndex(`${podUri}settings/publicTypeIndex.jsonld`, { listed: true });
await storage.write(`${podPath}settings/publicTypeIndex.jsonld`, serialize(publicTypeIndex));

const privateTypeIndex = generateTypeIndex(`${podUri}settings/privateTypeIndex.jsonld`, { listed: false });
await storage.write(`${podPath}settings/privateTypeIndex.jsonld`, serialize(privateTypeIndex));

// Create default ACL files. Each .acl is written inside the container it
// protects, so `acl:accessTo` is always './' (resolved against the .acl's
// own URL by the parser — see #428).
//
// The owner WebID is also written relatively (#430), derived from the
// absolute `webId` and the .acl's location within the pod by
// `relativizeOwnerWebId`. This works for any profile layout (modern
// `profile/card.jsonld#me`, legacy `profile/card#me`, custom shapes) and
// falls back to the absolute WebID for foreign owners. Together this
// keeps the on-disk pod portable across hostnames.
const owner = aclBase => relativizeOwnerWebId(webId, podUri, aclBase);

const rootAcl = generateOwnerAcl('./', owner(''), true);
await storage.write(`${podPath}.acl`, serializeAcl(rootAcl));

const privateAcl = generatePrivateAcl('./', owner('private/'));
await storage.write(`${podPath}private/.acl`, serializeAcl(privateAcl));

const settingsAcl = generatePrivateAcl('./', owner('settings/'));
await storage.write(`${podPath}settings/.acl`, serializeAcl(settingsAcl));

// publicTypeIndex: public read, overrides the private default inherited
// from /settings/. This is a resource ACL (lives at .../publicTypeIndex.jsonld.acl),
// whose base URL is /settings/ — same depth as `settings/.acl` for the
// owner reference.
const publicTypeIndexAcl = generateOwnerAcl('./publicTypeIndex.jsonld', owner('settings/'), false);
await storage.write(`${podPath}settings/publicTypeIndex.jsonld.acl`, serializeAcl(publicTypeIndexAcl));

const inboxAcl = generateInboxAcl('./', owner('inbox/'));
await storage.write(`${podPath}inbox/.acl`, serializeAcl(inboxAcl));

const publicAcl = generatePublicFolderAcl('./', owner('public/'));
await storage.write(`${podPath}public/.acl`, serializeAcl(publicAcl));

// Profile documents must be publicly readable for WebID verification
const profileAcl = generatePublicFolderAcl('./', owner('profile/'));
await storage.write(`${podPath}profile/.acl`, serializeAcl(profileAcl));

// Initialize storage quota if configured
if (defaultQuota > 0) {
await initializeQuota(name, defaultQuota);
}

// Optional: provision a Schnorr secp256k1 owner key in /private/.
// Phase 1 of #437. See src/keys/provision.js for the design notes.
// Owner-key file is written last (after the rest of the pod
// structure exists) so a failure during ACL setup doesn't leave a
// secret on disk without proper protection. The keypair itself was
// generated up-front for profile injection; this step persists it.
// Throw on write failure so the caller's cleanup path runs and the
// pod isn't left with a phantom `ownerKey` in the response that
// doesn't correspond to any on-disk file.
//
// Strict `=== true` (not just truthy) so a misconfigured caller
// passing `'true'` / `1` / etc. doesn't silently activate. Matches
// handleCreatePod's HTTP-side check on the body field.
let ownerKey;
if (options.provisionKeys === true) {
ownerKey = provisionOwnerKey({ controllerWebId: webId });
if (ownerKey) {
const ok = await storage.write(
`${podPath}private/privkey.jsonld`,
JSON.stringify(ownerKey.document, null, 2),
{ mode: 0o600 }
);
if (!ok) {
throw new Error(
`Failed to write owner key file at ${podPath}private/privkey.jsonld`
);
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 2285d05: reordered both createPodStructure (multi-user) and createRootPodStructure (single-user) to write /private/privkey.jsonld BEFORE the WebID profile. The keypair is still generated in memory up-front so the VM can be injected; the persistence now happens before the profile that advertises it. Worst-case crash leaves an orphan secret file (easy to delete) rather than an orphan VM in a published profile.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 2285d05: reordered both createPodStructure (multi-user) and createRootPodStructure (single-user) to write /private/privkey.jsonld BEFORE the WebID profile. The keypair is still generated in memory up-front so the VM can be injected; the persistence now happens before the profile that advertises it. Worst-case crash leaves an orphan secret file (easy to delete) rather than an orphan VM in a published profile.

Comment thread src/server.js Outdated
Comment on lines 1048 to 1118
@@ -1090,13 +1100,11 @@ export function createServer(options = {}) {
const profileAcl = generatePublicFolderAcl('./', owner('profile/'));
await storage.write('/profile/.acl', serializeAcl(profileAcl));

// Optional: provision a Schnorr secp256k1 owner key in /private/.
// Phase 1 of #437. See src/keys/provision.js for the design notes.
// Throw on write failure so single-user startup fails loud rather
// than logging "Provisioned …" against a missing on-disk file.
let ownerKey;
if (provisionKeysEnabled) {
ownerKey = provisionOwnerKey({ controllerWebId: webId });
// Owner-key file is written last. The keypair itself was generated
// up-front for profile injection; this step persists it. Throw on
// write failure so single-user startup fails loud rather than
// logging "Provisioned …" against a missing on-disk file.
if (ownerKey) {
const ok = await storage.write(
'/private/privkey.jsonld',
JSON.stringify(ownerKey.document, null, 2),
@@ -1109,8 +1117,10 @@ export function createServer(options = {}) {
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 2285d05: reordered both createPodStructure (multi-user) and createRootPodStructure (single-user) to write /private/privkey.jsonld BEFORE the WebID profile. The keypair is still generated in memory up-front so the VM can be injected; the persistence now happens before the profile that advertises it. Worst-case crash leaves an orphan secret file (easy to delete) rather than an orphan VM in a published profile.

Comment thread test/keys-provision-lws-cid.test.js Outdated
Comment on lines +163 to +164
assert.match(result.error || '', /signature/i,
'error should mention signature failure');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 2285d05: tightened the negative test assertion to /signature verification failed/ — matches the specific verifier error path rather than any string containing 'signature'. Fixes the false-positive on JWT-claim errors that happen to mention signing.

Comment thread src/webid/profile.js
Comment on lines +130 to +134
...(ownerVm && {
verificationMethod: [ownerVm],
authentication: [ownerVm['@id']],
assertionMethod: [ownerVm['@id']]
})

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 2285d05: added a JSDoc note on generateProfileJsonLd flagging the helper as fresh-pod-only when ownerVm is provided (overwrites verificationMethod / authentication / assertionMethod). Future re-provisioning callers should merge first.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already addressed in 2285d05 (the previous round) — generateProfileJsonLd now has a JSDoc note flagging the helper as fresh-pod-only when ownerVm is provided. The line number drift after that edit re-anchored this comment; the doc warning is in place a few lines above the line the comment is now pointing at.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already addressed — generateProfileJsonLd has the JSDoc note flagging fresh-pod-only since 2285d05. Copilot keeps re-anchoring to the new line numbers; the note is in place a few lines above where this comment is now pointing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already addressed — the JSDoc note flagging generateProfileJsonLd as fresh-pod-only when ownerVm is set has been in place since 2285d05. Copilot keeps re-anchoring this comment to new line numbers; the doc is at the function definition, several lines above where the anchor now points.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already addressed in earlier commits — Copilot's re-anchoring to new line numbers but the substantive fixes are in place (asymmetry documented in provisionOwnerKey JSDoc + new test in e8a268b, hexToBytes validates in e8a268b, generateProfileJsonLd fresh-pod note merged into a single JSDoc block in the next push).

…#444)

Four batched pickups from Copilot.

1. provisionOwnerKey took three overlapping inputs (webId,
   controllerWebId, controller) with subtle precedence rules.
   Drop the legacy controllerWebId alias; rename `controller` to
   `documentController` so the two knobs (webId for VM, document
   override for the doc) are unambiguous. All in-tree callers
   migrated this PR — no external break.

2. Pod creation wrote the WebID profile (advertising the owner-key
   VM) BEFORE /private/privkey.jsonld. Crash between the two would
   leave the WebID permanently advertising an authentication method
   whose secret was never persisted. Reorder: privkey first, profile
   last. Worst-case crash now leaves an orphan secret file (easy to
   delete) rather than an orphan VM in a public profile. Same fix
   in createPodStructure (multi-user) and createRootPodStructure
   (single-user).

3. The negative LWS-CID test asserted /signature/i — too loose; JWT
   errors for unrelated reasons (expired, malformed) also mention
   "signature". Tighten to /signature verification failed/ so the
   test actually proves the signature check did real work.

4. generateProfileJsonLd unconditionally overwrites the
   verificationMethod / authentication / assertionMethod arrays
   when ownerVm is set. Fine for fresh-pod creation (the only
   current caller), but a future re-provisioning command would
   silently drop operator-added VMs. Add a JSDoc note flagging the
   helper as fresh-pod-only and pointing at the right merge pattern
   for re-generation.

854/854 tests pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread src/keys/provision.js Outdated
const document = buildOwnerKeyDocument({
publicHex,
secretHex,
controller: documentController ?? didNostrFromPublicHex(publicHex)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the next push: pass documentController through verbatim and let buildOwnerKeyDocument apply its existing did:nostr default. One source of truth for the default.

provisionOwnerKey was computing didNostrFromPublicHex(publicHex) as a
fallback before passing the controller to buildOwnerKeyDocument — but
buildOwnerKeyDocument already applies the same default. Two sources
of truth for the same default; trim provisionOwnerKey's call site to
pass documentController through verbatim. Behaviour unchanged.

854/854 tests pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread src/handlers/container.js Outdated
Comment on lines +209 to +220
if (ownerKey) {
const ok = await storage.write(
`${podPath}private/privkey.jsonld`,
JSON.stringify(ownerKey.document, null, 2),
{ mode: 0o600 }
);
if (!ok) {
throw new Error(
`Failed to write owner key file at ${podPath}private/privkey.jsonld`
);
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the next push: reordered to write privkey AFTER ACLs (born under owner-only WAC) but BEFORE profile (orphan-VM property preserved). Single block-comment in both call sites now spells out both ordering concerns. The previous fix had inadvertently solved one issue while opening another; this nests them correctly.

Comment thread src/server.js Outdated
Comment on lines +1060 to +1071
if (ownerKey) {
const ok = await storage.write(
'/private/privkey.jsonld',
JSON.stringify(ownerKey.document, null, 2),
{ mode: 0o600 }
);
if (!ok) {
throw new Error(
'Failed to write owner key file at /private/privkey.jsonld'
);
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the next push: reordered to write privkey AFTER ACLs (born under owner-only WAC) but BEFORE profile (orphan-VM property preserved). Single block-comment in both call sites now spells out both ordering concerns. The previous fix had inadvertently solved one issue while opening another; this nests them correctly.

Address Copilot pickup on #444. The previous reorder (privkey before
profile) addressed orphan-VM-on-crash but introduced a separate
issue: privkey was written *before* the ACL tree existed, so during
the gap between privkey write and /private/.acl write the secret
file had no WAC protection. jss's deny-by-default since #f43ecdf
mitigates real exposure to 401, but relying on a security default
holding is fragile — defence-in-depth beats it.

Right ordering: ACLs (already in place) → privkey (born under
owner-only WAC) → profile (still last so orphan-VM property
preserved on crash). Same fix in createPodStructure (multi-user)
and createRootPodStructure (single-user, where the exposure is more
acute since the pod URL is the server origin).

Both call sites now have a single block-comment block explaining the
two distinct concerns the ordering serves: WAC vacuum + orphan VM.

854/854 tests pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comment thread test/keys-provision.test.js Outdated
Comment on lines 121 to 131
it('accepts an explicit controller override (Phase 1 backward-compat)', () => {
// Tests / fixtures that need the legacy WebID-controller shape can
// pass `controller` explicitly. Phase 1-style call sites and any
// future caller wanting a non-default controller stay supported.
const doc = buildOwnerKeyDocument({
publicHex: args.publicHex,
secretHex: args.secretHex,
controller: args.controllerWebId
});
assert.strictEqual(doc.controller, args.controllerWebId);
assert.doesNotMatch(doc.controller, /^did:nostr:/);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: renamed the test to '(e.g. WebID, urn:, custom DID method)' so future readers don't go looking for legacy support that isn't there. The mechanism is the same (the explicit controller override) but now the name describes what it actually does.

Comment thread src/keys/provision.js
Comment on lines +247 to 270
export function provisionOwnerKey({ webId, documentController }) {
if (typeof webId !== 'string' || !webId) {
throw new Error('provisionOwnerKey: webId required');
}
const { publicHex, secretHex } = generateOwnerKeypair();
const document = buildOwnerKeyDocument({ controllerWebId, publicHex, secretHex });
// Pass `documentController` through verbatim — buildOwnerKeyDocument
// already defaults to `did:nostr:<publicHex>` when none is given, so
// duplicating the fallback here would just create two sources of
// truth for the default controller.
const document = buildOwnerKeyDocument({
publicHex,
secretHex,
controller: documentController
});
const vm = buildOwnerVerificationMethod({ webId, publicHex });
return {
document,
vm,
publicHex,
secretHex,
publicMultibase: document.publicKeyMultibase
publicMultibase: document.publicKeyMultibase,
didNostr: didNostrFromPublicHex(publicHex)
};
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: documented the asymmetry explicitly in provisionOwnerKey's JSDoc as a 'two controllers, on purpose' design — vm.controller identifies who in WebID-land presents the VM (always the pod owner), document.controller identifies the key in its own right (default did:nostr, overridable). Added a unit test pinning the asymmetric behaviour as a documented expectation. Sync isn't appropriate here because the two fields are semantically distinct in CID v1; the JSDoc points callers wanting symmetric controllers at buildOwnerVerificationMethod for a separate construction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already addressed in earlier commits — Copilot's re-anchoring to new line numbers but the substantive fixes are in place (asymmetry documented in provisionOwnerKey JSDoc + new test in e8a268b, hexToBytes validates in e8a268b, generateProfileJsonLd fresh-pod note merged into a single JSDoc block in the next push).

Comment thread src/keys/provision.js
Comment on lines +316 to +320
function hexToBytes(hex) {
const out = new Uint8Array(hex.length / 2);
for (let i = 0; i < out.length; i++) out[i] = parseInt(hex.slice(i * 2, i * 2 + 2), 16);
return out;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: hexToBytes now validates with the same /^[0-9a-fA-F]+$/ + even-length check the sibling helpers use. The current caller is safe (always passes freshly-generated hex), but a shared helper should never silently corrupt — caught for any future caller.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already addressed in earlier commits — Copilot's re-anchoring to new line numbers but the substantive fixes are in place (asymmetry documented in provisionOwnerKey JSDoc + new test in e8a268b, hexToBytes validates in e8a268b, generateProfileJsonLd fresh-pod note merged into a single JSDoc block in the next push).

Three batched pickups from Copilot.

1. Test name '(Phase 1 backward-compat)' was misleading after the
   controllerWebId alias was dropped — there's no backward-compat
   path, just the forward-looking explicit-controller override knob.
   Renamed.

2. provisionOwnerKey's documentController doesn't propagate to
   vm.controller — VM stays pinned to webId. The previous JSDoc
   didn't surface this; documented as intentional ('two controllers,
   on purpose') with the rationale: vm.controller identifies who in
   WebID-land presents the VM (always the pod owner), document.
   controller identifies the key in its own right (default did:nostr,
   overridable). Added a unit test pinning the asymmetry as expected
   behaviour with an example documentController value.

3. hexToBytes silently corrupted on invalid input (parseInt('zz', 16)
   → NaN → 0). Added the same up-front validation pattern the sibling
   helpers in this file use. Today's only caller passes valid hex,
   but the helper is shared infrastructure and should validate.

855/855 tests pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread src/webid/profile.js Outdated
Comment on lines +27 to +39
/**
* NOTE on `ownerVm`: when provided, this helper unconditionally
* overwrites `verificationMethod`, `authentication`, and
* `assertionMethod` with single-element arrays. That's correct for
* the only current caller (fresh-pod creation, where there's nothing
* to overwrite), but if a future "rotate-keys" / re-provisioning
* command ever calls this helper to regenerate an existing profile,
* it will silently drop any VMs / proof-purpose references the
* operator added by hand. Either pass the existing profile through
* a merge step before calling this, or treat this helper as
* fresh-pod-only. See #444 review.
*/
export function generateProfileJsonLd({ webId, name, podUri, issuer, ownerVm = null }) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: merged the two JSDoc blocks. The fresh-pod-only note lived in a separate block above the original, which shadowed the original's @param entries. Now a single block with all params (including ownerVm with the warning inline) attached to the function definition.

The fresh-pod-only note added in 2285d05 lived in a separate JSDoc
block above the original — JSDoc tooling only associates the
closest block to the symbol, so the original @param entries for
webId/name/podUri/issuer/@returns were getting orphaned. Merged
into one block with the ownerVm @param + the fresh-pod warning
inline.

855/855 tests pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: --provision-keys Phase 2 — wire the owner key into the WebID profile, close the LWS-CID loop (#437)

2 participants