Skip to content

Commit 9c2e001

Browse files
Address copilot pass 6 on #4
Three real findings: 1. entryMatchesId only did exact string equality, missing the relative-IRI case (line 639). JSON-LD profiles often write VM ids as `"#lws-key-1"` which resolve against the document URL. Without absolutization, fragment-collision detection would walk PAST an existing relative entry as if free, then merge would create a duplicate. Now resolves both sides against baseUrl before comparing. Verified across absolute/relative/object- wrapped forms. 2. authentication de-dupe had the same blind spot (line 631) — could push an absolute IRI even when an equivalent "#fragment" already existed. Now absolutizes both sides via the same helper. 3. PATCH-failure path left the test UI in a stale "ready to test" state if a prior run had succeeded (line 500). Could mislead a user into hitting Test with a kid the server may not have. Now clears lastVmKid, hides testSection, resets testResult in the catch block.
1 parent 2116c05 commit 9c2e001

1 file changed

Lines changed: 36 additions & 8 deletions

File tree

doctor.js

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,15 @@ patchButton.addEventListener('click', async () => {
497497
} catch (err) {
498498
patchResult.className = 'patch-result error';
499499
patchResult.textContent = `Failed: ${err.message || err}`;
500+
// Reset everything that depended on the patch having succeeded.
501+
// Otherwise a UI that already showed the test section from a
502+
// prior successful run would still claim "ready to test" with a
503+
// stale kid against a now-uncertain server state.
500504
memPrivKey = null;
505+
lastVmKid = null;
506+
testSection.hidden = true;
507+
testResult.textContent = '';
508+
testResult.className = 'test-result';
501509
}
502510
});
503511

@@ -576,7 +584,7 @@ function chooseFragmentAndBuildVm({ privKey, profile, webId, controller }) {
576584

577585
for (let n = 1; n <= 99; n++) {
578586
const candidateId = `${docUrl}#lws-key-${n}`;
579-
const existing = vms.find((v) => entryMatchesId(v, candidateId));
587+
const existing = vms.find((v) => entryMatchesId(v, candidateId, docUrl));
580588
if (!existing) {
581589
const result = buildEs256kVerificationMethod({
582590
privKey, webId, controller, fragment: `lws-key-${n}`,
@@ -615,30 +623,50 @@ function chooseFragmentAndBuildVm({ privKey, profile, webId, controller }) {
615623
*/
616624
function mergeVerificationMethod(profile, vm) {
617625
const out = { ...profile };
626+
const baseUrl = stripHashLocal(vm.id);
618627
const vms = Array.isArray(out.verificationMethod) ? [...out.verificationMethod]
619628
: out.verificationMethod ? [out.verificationMethod]
620629
: [];
621-
const idx = vms.findIndex((v) => entryMatchesId(v, vm.id));
630+
const idx = vms.findIndex((v) => entryMatchesId(v, vm.id, baseUrl));
622631
if (idx >= 0) vms[idx] = vm;
623632
else vms.push(vm);
624633
out.verificationMethod = vms;
625634

626635
const auth = Array.isArray(out.authentication) ? [...out.authentication]
627636
: out.authentication ? [out.authentication]
628637
: [];
629-
if (!auth.some((a) => (typeof a === 'string' ? a : a?.['@id'] || a?.id) === vm.id)) {
630-
auth.push(vm.id);
631-
}
638+
// De-dupe against the absolutized form so a pre-existing relative
639+
// entry like "#lws-key-1" is recognized as the same as the absolute
640+
// URI we're about to push.
641+
const exists = auth.some((a) => {
642+
const raw = typeof a === 'string' ? a : (a?.['@id'] || a?.id);
643+
if (typeof raw !== 'string') return false;
644+
return absolutizeLocal(raw, baseUrl) === vm.id;
645+
});
646+
if (!exists) auth.push(vm.id);
632647
out.authentication = auth;
633648
return out;
634649
}
635650

636-
function entryMatchesId(entry, id) {
637-
if (typeof entry === 'string') return entry === id;
638-
if (entry && typeof entry === 'object') return (entry.id || entry['@id']) === id;
651+
function entryMatchesId(entry, id, baseUrl) {
652+
// Handle relative IRIs the way JSON-LD does: resolve against the
653+
// document URL before comparing. This lets us recognize existing
654+
// entries written as "#lws-key-1" as equivalent to the absolute
655+
// form we generate.
656+
const resolve = (s) => (typeof s === 'string' ? absolutizeLocal(s, baseUrl) : s);
657+
if (typeof entry === 'string') return resolve(entry) === resolve(id);
658+
if (entry && typeof entry === 'object') {
659+
const raw = entry.id || entry['@id'];
660+
return typeof raw === 'string' && resolve(raw) === resolve(id);
661+
}
639662
return false;
640663
}
641664

665+
function absolutizeLocal(u, base) {
666+
if (!u) return u;
667+
try { return new URL(u, base).toString(); } catch { return u; }
668+
}
669+
642670
function sameJwk(a, b) {
643671
// Compare the public-key material, not auxiliary fields like `kid`,
644672
// `alg`, or `use`. Two VMs are "the same key" iff x and y match.

0 commit comments

Comments
 (0)