Skip to content

Verify quotes with Fedify 2.5 APIs - #435

Merged
dahlia merged 1 commit into
hackers-pub:mainfrom
dahlia:refactor/remove-quote-verification-workarounds
Oct 7, 2026
Merged

dahlia merged 1 commit into
hackers-pub:mainfrom
dahlia:refactor/remove-quote-verification-workarounds

Conversation

@dahlia

@dahlia dahlia commented Oct 7, 2026

Copy link
Copy Markdown
Member

Fedify 2.5.0-dev.2355+c92a1cb includes fedify-dev/fedify#1245, allowing quote verification to use resolved objects and compatibility options instead of request normalization. This preserves quoteUrl and multiple-author support. Authorization aliases require a signed author approval or a matching, unrevoked DB grant.

Capture the signed Accept.result IRI before dereferencing so a fetched object's ID cannot replace it. Pass document and context loaders separately, keeping JSON-LD context loading independent of authorization fetching.

Fixes #429.

Use Fedify 2.5.0-dev.2355 verification options to preserve quoteUrl
and multiple-author compatibility without normalizing requests.
Verify authorization aliases against signed approvals or unrevoked
stored grants, and pass document and context loaders separately.

Capture the signed Accept result IRI before dereferencing so a fetched
object cannot replace the authorization identity. Cover redirected
results, stored grant bindings, DB errors and context loaders.

Fixes hackers-pub#429
fedify-dev/fedify#1245

Assisted-by: Codex:gpt-6.1-sol
Assisted-by: Claude Code:claude-fable-5-1
@dahlia dahlia self-assigned this Oct 7, 2026
@dahlia dahlia added enhancement New feature or request federation ActivityPub federation labels Oct 7, 2026
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c8ad2262-f466-493e-9067-f059661d6f29
📥 Commits

Reviewing files that changed from the base of the PR and between b44225e and 952ba02.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (11)
  • federation/inbox/dispatch.test.ts
  • federation/inbox/dispatch.ts
  • federation/inbox/quote.test.ts
  • federation/inbox/quote.ts
  • federation/interaction-controls.test.ts
  • federation/services.ts
  • models/post.remote.test.ts
  • models/post/remote.ts
  • models/services.ts
  • package.json
  • pnpm-workspace.yaml

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

Quote request handling now verifies resolved inputs without rewriting request or instrument identifiers. Inbox acceptance uses the selected authorization ID. Shared verification covers locally issued authorizations and separate document and context loaders.

Changes

Quote authorization verification

Layer / File(s) Summary
Shared verification contract
models/services.ts, federation/services.ts, federation/interaction-controls.test.ts, package.json, pnpm-workspace.yaml
The service contract and implementation add stored quote-authorization verification with separate document and context loaders. Tests cover stored evidence and loader behavior. The @fedify/* catalog versions change to 2.5.0-dev.2355.
Inbox quote request and acceptance
federation/inbox/quote.ts, federation/inbox/dispatch.ts, federation/inbox/dispatch.test.ts, federation/inbox/quote.test.ts, federation/interaction-controls.test.ts
Quote request verification uses the original request and resolved target and instrument. Acceptance uses the selected authorization ID for verification and persistence. Tests cover parsed request values and a dereferenced authorization whose ID differs from the Accept’s ID.
Locally issued authorization persistence
models/post/remote.ts, models/post.remote.test.ts
Persistence delegates verification of locally issued authorizations with the captured authorization ID, expected interaction details, and matching unrevoked-row evidence. Tests cover invalid stored records and database errors.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 952ba

Quote authorization checks appear ready to merge after normal checks; no actionable merge-blocking issue remains.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive The summary supports the core #429 changes: quote verification uses resolved inputs and accepts either quote reference and any valid attribution; authorization checks preserve the signed result ID and… Focused evidence from the quote handler and its tests is needed to confirm database resolution and requester-origin checks, and coverage for conflicting quote references and multiple attributions.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: updating quote verification to use Fedify 2.5 APIs.
Description check ✅ Passed The description explains the quote-verification changes, authorization checks, loader handling, and linked issue.
Out of Scope Changes check ✅ Passed The changed quote-verification code, authorization checks, regression tests, loader changes, and Fedify version updates support #429. The summary identifies no unrelated changes.
Full details: Linked Issues check

Explanation

The summary supports the core #429 changes: quote verification uses resolved inputs and accepts either quote reference and any valid attribution; authorization checks preserve the signed result ID and validate signed or stored grants; document and context loaders are separate. The summary also reports regression tests for result-ID mismatches, missing attribution, stored grant bindings, and loader behavior. It does not establish regression coverage for conflicting quote references or multiple attributions. It also does not establish that local targets and share wrappers still resolve through the database or that requester-origin checks remain. These requirements cannot be confirmed from the available summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dahlia
dahlia merged commit cbf6784 into hackers-pub:main Oct 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request federation ActivityPub federation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove quote verification workarounds after fedify-dev/fedify#1206

1 participant