Reject unsupported document URL schemes when private addresses are allowed by dahlia · Pull Request #1293 · fedify-dev/fedify

dahlia · 2026-10-11T07:44:06Z

The built-in document loaders checked URL schemes only inside validatePublicUrl(), and both skip that call when allowPrivateAddress is enabled. Relaxing address validation therefore also disabled scheme validation, letting ftp:, file:, and data: URLs reach the runtime's fetch(). The result depended on the runtime: Deno throws a plain TypeError that callers may treat as retryable, and Node.js actually loads data: URLs, so a redirect or alternate link could make the loader return a document of the remote server's choosing.

getDocumentLoader() and getAuthenticatedDocumentLoader() now reject anything other than HTTP and HTTPS with UrlError before fetching, independent of allowPrivateAddress. The checks run in the recursive load() in packages/vocab-runtime/src/docloader.ts and in validateUrl() in packages/fedify/src/utils/docloader.ts, which doubleKnock() calls before following a redirect. Every followed URL already passes through one of these, so redirects and alternate documents need no separate checks. HTTP(S) URLs with private addresses still work.

The guards are identical to the ones #1291 adds to 2.4-maintenance, so merging forward should only need the tests reconciled. Tests assert that unsupported URLs never reach fetch() directly, through redirects or Link header/HTML alternates, or after a double-knock fallback from either signature spec.

Fixes #1292.

The only HTTP(S) scheme check in the built-in document loaders lived inside validatePublicUrl(), which both getDocumentLoader() and getAuthenticatedDocumentLoader() skip when allowPrivateAddress is enabled. An option meant to relax address validation therefore also disabled scheme validation, letting ftp:, file:, data: and other URLs reach the runtime's fetch(). The outcome depended on the runtime: Deno throws a generic TypeError instead of UrlError, so callers can treat it as a retryable failure, while Node.js successfully loads data: URLs, so a redirect or alternate link could make the loader return an arbitrary document. Both loaders now reject non-HTTP(S) URLs with UrlError before fetching, whether or not private addresses are allowed. The check runs on every URL the loaders follow, so redirect targets (including those after a double-knock signature fallback) and alternate documents found via the Link header or HTML <link> elements are covered too. HTTP(S) URLs with private addresses remain allowed. The guards mirror the ones added to 2.4-maintenance by the pull request below to keep the merge forward simple. Fixes fedify-dev#1292 fedify-dev#1291 Assisted-by: Codex:gpt-6-astra Assisted-by: Claude Code:claude-opus-5-5

chatgpt-codex-connector · 2026-10-11T07:44:16Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-11T07:47:28.105792Z	`b889462`	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

coderabbitai · 2026-10-11T07:44:27Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 8362a6f0-b8cc-48d7-befb-0142565e9a52

📥 Commits

Reviewing files that changed from the base of the PR and between 8f913f0 and b889462.

📒 Files selected for processing (7)

CHANGES.md
changes.d/fedify/document-url-schemes.md
changes.d/vocab-runtime/document-url-schemes.md
packages/fedify/src/utils/docloader.test.ts
packages/fedify/src/utils/docloader.ts
packages/vocab-runtime/src/docloader.test.ts
packages/vocab-runtime/src/docloader.ts

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

📝 Walkthrough

Walkthrough

Both document loaders now reject unsupported URL schemes with UrlError before fetching, even when private-address access is enabled. Regression tests cover direct URLs, redirects, and alternate document targets. Private-address HTTP(S) URLs remain allowed.

Changes

Document URL Validation

Layer / File(s)	Summary
Fedify authenticated document loader `packages/fedify/src/utils/docloader.ts`, `packages/fedify/src/utils/docloader.test.ts`, `changes.d/fedify/document-url-schemes.md`, `CHANGES.md`	The loader rejects non-HTTP(S) schemes before private-address validation. Tests cover direct loads, redirects, signature fallback redirects, and HTML or Link-header alternates. The changelog records the behavior.
Vocab-runtime document loader `packages/vocab-runtime/src/docloader.ts`, `packages/vocab-runtime/src/docloader.test.ts`, `changes.d/vocab-runtime/document-url-schemes.md`, `CHANGES.md`	The loader rejects non-HTTP(S) schemes and uses the parsed URL’s `href` for accepted URLs. Tests cover direct loads, redirects, and HTML or Link-header alternates. The changelog records the behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: sij411

Merge Risk: ⚪ Minimal · up to b8894

The change appears ready to merge after normal checks. No actionable risk remains identified.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (3 skipped: 3…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: rejecting unsupported document URL schemes when private-address access is enabled.
Description check		The description directly explains the affected loaders, the HTTP(S)-only validation, the allowPrivateAddress behavior, and the regression-test coverage.
Linked Issues check		Issue `#1292` requires both built-in loaders to reject non-HTTP(S) schemes with `UrlError` before fetch, including when `allowPrivateAddress` is enabled. `packages/vocab-runtime/src/docloader.ts` checks…
Out of Scope Changes check		The source changes directly implement issue `#1292`. The new tests verify the required rejection paths and no-fetch behavior. The changelog entries document the same loader behavior. No unrelated change…

Full details: Docstring Coverage

Explanation

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

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · 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.}

codecov · 2026-10-11T07:51:36Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines	Coverage Δ
packages/fedify/src/utils/docloader.ts	`100.00% <100.00%> (ø)`
packages/vocab-runtime/src/docloader.ts	`96.51% <100.00%> (+0.44%)`	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dahlia self-assigned this Oct 11, 2026

dahlia added component/vocab Activity Vocabulary related component/federation Federation object related labels Oct 11, 2026

dahlia requested review from 2chanhaeng and sij411 October 11, 2026 07:54

2chanhaeng approved these changes Oct 11, 2026

View reviewed changes

dahlia merged commit bacabf0 into fedify-dev:2.0-maintenance Oct 11, 2026
17 checks passed

dahlia deleted the bugfix/document-url-schemes branch October 11, 2026 13:17

dahlia linked an issue Oct 11, 2026 that may be closed by this pull request

Reject unsupported document URL schemes with private addresses allowed #1292

Closed

Navigation Menu

Repository navigation

Reject unsupported document URL schemes when private addresses are allowed - #1293

Reject unsupported document URL schemes when private addresses are allowed#1293
dahlia merged 1 commit into
fedify-dev:2.0-maintenancefrom
dahlia:bugfix/document-url-schemes

dahlia commented Oct 11, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 11, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 11, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Oct 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dahlia commented Oct 11, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

coderabbitai Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Oct 11, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chatgpt-codex-connector Bot commented Oct 11, 2026 •

edited

Loading

coderabbitai Bot commented Oct 11, 2026 •

edited

Loading