Support FEP-fe34 cryptographic origins for portable objects#926
Conversation
Treat FEP-ef61 portable ActivityPub IDs and DID URLs as sharing an FEP-fe34 cryptographic origin when their DID components match. This lets lookupObject(), generated vocabulary accessors, and did:key object proofs handle portable objects without relaxing existing same-origin checks for other IRI schemes. Restrict proof-based portable attribution authentication to verification methods that are themselves in the matching cryptographic origin, so a remote HTTPS key cannot claim a victim DID controller and authenticate that DID's portable objects. fedify-dev#829 fedify-dev#926 Assisted-by: Codex:gpt-5.5
✅ Deploy Preview for fedify-json-schema canceled.
|
|
@codex review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds FEP-fe34 cryptographic-origin support for portable ChangesFEP-fe34 origin support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request implements FEP-fe34 cross-origin checks for FEP-ef61 portable ActivityPub IDs and DID URLs across the @fedify/fedify, @fedify/vocab, and @fedify/vocab-runtime packages. It introduces getFe34Origin() and haveSameFe34Origin() helpers to compute and compare cryptographic origins (using the DID component) alongside traditional web origins, and updates verifyObject() and lookupObject() to leverage these checks. The review feedback suggests improving DID normalization by ensuring that DID method names are consistently converted to lowercase to prevent case-sensitivity mismatches during origin comparisons, and recommends reusing this normalized logic when resolving portable cryptographic origins.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44441b5291
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Normalize DID method names when deriving FE34 origins so portable ActivityPub IDs and DID URLs compare according to DID Core casing rules. Reuse that validation path for portable authorities instead of maintaining a separate normalization shortcut. Keep exact DID attributions protected by the same verification-method origin check used for portable attributions. This prevents an attacker-hosted HTTPS key document from claiming a DID controller and authenticating objects attributed directly to that DID. fedify-dev#926 (comment) fedify-dev#926 (comment) fedify-dev#926 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request implements cross-origin checks for FEP-fe34 cryptographic origins, supporting FEP-ef61 portable ActivityPub IDs and DID URLs. It introduces the helper functions getFe34Origin() and haveSameFe34Origin() in the vocabulary runtime to resolve and compare web origins and cryptographic DID origins. These helpers are integrated into verifyObject(), lookupObject(), and property accessors to treat matching portable IDs and DID controllers as same-origin. Comprehensive tests and documentation updates have also been added. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a436473f37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Apply the same percent-encoding normalization to FE34 DID origins that portable URI canonicalization already uses. This lets equivalent DID authorities compare as the same cryptographic origin even when one form uses percent escapes for unreserved characters. fedify-dev#926 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
@codex review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
sij411
left a comment
There was a problem hiding this comment.
I found hasCryptographicOrigin() does similar job to getFe34Origin() 's parsePortableIri() and DID_SCHEME_PATTERN.test(input) from @fedify/vocab-runtime but only a small part can be shared so I don't think it's a big deal.
| for await (const proof of object.getProofs(options)) { | ||
| const key = await verifyProof(jsonLd, proof, options); | ||
| if (key === null) return null; | ||
| if (proof.verificationMethodId == null) return null; |
There was a problem hiding this comment.
if (proof.verificationMethodId == null) return null; is not useful. Because if proof.verificationMethodId == null, then verifyProof() will return null and verifyObject() will return null, already.
| } | ||
|
|
||
| function hasCryptographicOrigin(iri: string): boolean { | ||
| return /^did:/i.test(iri) || /^ap(?:\+ef61)?:\/\//i.test(iri); |
There was a problem hiding this comment.
This local RegExps are also defined in packages/vocab-runtime/src/url.ts. Import them and reuse for consistency and maintainability.
Closes #829.
Why
Fedify already had two pieces of the portable-object story: FEP-ef61 URI parsing and local
did:keyverification method resolution for FEP-8b32 proofs. What was still missing was the policy layer between them. A portable object whose ID isap://did:key:.../objects/1should be able to carry an embeddeddid:key:...#...verification method or a matching portable actor without being treated as cross-origin just because the schemes differ.This PR adds that bridge by comparing the FEP-fe34 origin of web URLs, portable
ap:/ap+ef61:IDs, and DID URLs. HTTP(S) IDs keep the existing web-origin behavior. Portable IDs and DID URLs use their DID component as the cryptographic origin.The proof path is intentionally narrower than the vocabulary lookup path. A proof can authenticate portable attributions by cryptographic origin only when the verification method ID is itself in the same cryptographic origin as the claimed controller. That keeps locally resolved
did:keyverification methods working while preventing an attacker-hosted HTTPS key document from claiming a victim DID controller and signing for that DID's portable objects.How
getFe34Origin()andhaveSameFe34Origin(). They compare ordinary HTTP(S) origins and DID-based cryptographic origins through one helper.at://.Testing
The test coverage includes matching and mismatched portable DID origins, multiple same-origin portable attributions, same-authority non-FE34 IRIs, and the HTTPS-key spoofing case described above.
Verified with: