fix: expose authenticated HTTP module redirects - #3839
Conversation
HTTP module imports can follow a valid redirect into an interactive sign-in page, leaving the cache to misclassify the final HTML response. Preserve the guarded redirect flow while carrying only the last fully admitted hop's status and sanitized destination into the diagnostic. Constraint: Redirect following, egress validation, credential stripping, retry policy, and endpoint authentication remain unchanged. Rejected: Stop at the first redirect | valid CDN and package redirects must continue to work. Rejected: Retry authentication redirects | the boundary is deterministic and requires project-owner migration. Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep redirect destinations sanitized before logs or public errors, and never notify observers about a destination the egress guard blocks. Tested: RED/GREEN redirect observer, blocked-destination, and HTTP cache regressions; 5 focused tests/134 steps; 3929 pre-push tests/30091 steps; check, lint, fmt, anti-slop, test-typecheck, pinned API docs, public docs, release checks. Not-tested: Project-owner migration of the private ResponsiveImage import. Related: veryfront/veryfront-issue-inbox#177 Related: veryfront/veryfront-issue-inbox#240
The redirect observer runs after the destination response is available, so an observer rejection must not strand that response body. Record the cleanup and error-propagation contract with a cancel-counting regression. Constraint: Preserve the observer error while releasing the unread response body. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep observer-failure cleanup covered when redirect notification timing changes. Tested: Focused redirect/egress/cache matrix, 5 tests/135 steps; check, lint, fmt, diff check. Related: veryfront/veryfront-issue-inbox#177
|
Warning Review limit reached
Next review available in: 56 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e53d2e33a9
ℹ️ 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".
Redirect destinations can carry one-time codes or signed credentials in path segments, and ordinary redirects do not prove authentication. Reduce neutral destinations to their origin, retain only the canonical sign-in marker, and choose guidance from that bounded classification. Constraint: Preserve the observed HTTP status and the known sign-in diagnosis without storing untrusted path, query, fragment, or userinfo data. Rejected: Reuse generic URL sanitization | it intentionally preserves path components that can contain credentials. Rejected: Label every redirected HTML response as authentication | canonical and moved-module redirects need neutral guidance. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Do not add arbitrary redirect path components back to logs or public errors. Tested: RED/GREEN moved-module and path-credential redirects; sign-in path/query redaction; focused matrix 5 tests/136 steps; check, lint, fmt, anti-slop, test-typecheck, diff check. Related: veryfront/veryfront-issue-inbox#177
|
@codex review exact head |
kojiwakayama
left a comment
There was a problem hiding this comment.
Addressed the linked review finding in b19240c.
kojiwakayama
left a comment
There was a problem hiding this comment.
Addressed the linked review finding in b19240c.
|
@coderabbitai review |
|
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
Outcome
Surface the deterministic authentication redirect that precedes an HTTP module HTML response. The loader now reports the last fully admitted redirect status and a bounded destination instead of misclassifying this failure as an esm.sh build or alias-resolution problem.
This is intentionally diagnostic-only. It does not retry the redirect, weaken endpoint authentication, or claim to restore the affected project render.
RED → GREEN
Safety
Verification
Follow-up
The project owner still needs to replace the public-domain ResponsiveImage import with an authenticated/content-addressed registry source. Issue #240 remains the structural product fix.
Related: veryfront/veryfront-issue-inbox#177
Related: veryfront/veryfront-issue-inbox#240