Skip to content

fix(tasks): route SafeFetch.server redirect ceiling through SECURITY_LIMITS#614

Merged
sroussey merged 1 commit into
mainfrom
claude/beautiful-mayer-voekyh
Jul 9, 2026
Merged

fix(tasks): route SafeFetch.server redirect ceiling through SECURITY_LIMITS#614
sroussey merged 1 commit into
mainfrom
claude/beautiful-mayer-voekyh

Conversation

@sroussey

@sroussey sroussey commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Route the server-side SafeFetch (Node/Bun) redirect ceiling through
SECURITY_LIMITS.safeFetchMaxRedirectHops instead of a local hardcoded 20,
matching what the browser sibling (SafeFetch.ts) already does.

Failure scenario

The consolidation into SECURITY_LIMITS in packages/util/src/limits.ts
covered the browser SafeFetch.ts but missed SafeFetch.server.ts — the
Node/Bun path that additionally does DNS pre-resolution and undici-Agent
connection pinning to defeat DNS rebinding. As a result the two entrypoints
could drift on their SSRF redirect-hop cap: raising or lowering the ceiling
in limits.ts would only affect the browser path, and the server would
silently keep the stale value. That is exactly the drift risk the
SECURITY_LIMITS constant was introduced to prevent.

Fix

  • Import SECURITY_LIMITS from @workglow/util (already a peer dependency
    of @workglow/tasks).
  • Replace const MAX_REDIRECT_HOPS = 20; with
    const MAX_REDIRECT_HOPS = SECURITY_LIMITS.safeFetchMaxRedirectHops;.

Two-line change; the constant name and its call site in the redirect loop
are unchanged, and SECURITY_LIMITS.safeFetchMaxRedirectHops is 20, so
this is behavior-preserving.

Test plan

  • bun run build:types — passes across all 36 packages.
  • Diff review: identical value (20), identical const name, import
    placement matches the browser sibling.
  • Manual smoke: server-side redirect chains beyond the ceiling still
    throw TOO_MANY_REDIRECTS.

Generated by Claude Code

…LIMITS

The server-side SafeFetch (Node/Bun) still hardcoded MAX_REDIRECT_HOPS = 20
even after PR #609 consolidated ceilings under SECURITY_LIMITS. The browser
sibling was migrated; the server path — the one that also does DNS pre-
resolution and undici-Agent pinning — was not. This restores the single
source of truth in packages/util/src/limits.ts.
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 62.82% 25678 / 40872
🔵 Statements 62.65% 26567 / 42400
🔵 Functions 64.03% 4874 / 7612
🔵 Branches 51.55% 12587 / 24416
File CoverageNo changed files found.
Generated in workflow #2653 for commit 9bf1060 by the Vitest Coverage Report Action

@sroussey
sroussey merged commit 92d6775 into main Jul 9, 2026
14 checks passed
@sroussey
sroussey deleted the claude/beautiful-mayer-voekyh branch July 9, 2026 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants