fix(csod): send bootstrap session cookies; repair fetchResponse() - #2769
fix(csod): send bootstrap session cookies; repair fetchResponse()#2769bigguy6883 wants to merge 1 commit into
Conversation
CSOD's search API requires the session cookies its bootstrap home page sets, in addition to the anonymous JWT. csod.mjs read that page with ctx.fetchText(), which discards response headers, so Set-Cookie was dropped and every search call arrived unauthenticated. Tenants that enforce this returned "HTTP 401 CSOD Unauthorized" and contributed zero jobs to a scan. Fixing it required repairing providers/_http.mjs first: fetchResponse() called the internal fetchWithTimeout() without its required `consume` argument, so it threw "consume is not a function" on every invocation. It had no callers, so nothing surfaced it. It now buffers the body inside the timer window and returns an equivalent Response — returning the live Response would reintroduce the stalled-body hang documented in that same file. Repeated Set-Cookie survives the reconstruction; null-body statuses are guarded. csod.mjs prefers ctx.fetchResponse and falls back to ctx.fetchText when a ctx predates it, so existing callers are unaffected. Tenants that set no cookies get no cookie header at all rather than an empty one. Verified live against careers-kln.csod.com: 401 with zero jobs before, 34 jobs after. Also corrects the csod header comment, which asserted that no session cookies were needed. Closes santifer#2768 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe HTTP helper now returns a readable reconstructed response with preserved metadata and headers. CSOD captures bootstrap session cookies and sends them with search requests when available, while older contexts retain the text-fetch fallback. ChangesCSOD session authentication
Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@providers/csod.mjs`:
- Line 188: Update the bootstrap request in the CSOD provider around
ctx.fetchResponse to pass redirect: 'error' alongside the existing HTML Accept
header, ensuring redirects are rejected before following any destination.
- Line 213: Update resolveConfig() and the cookie-handling flow to require HTTPS
before retrieving or replaying session cookies. Reject configurations whose CSOD
URLs use http: and ensure the cookie header is never sent for non-HTTPS
requests, while preserving existing HTTPS behavior.
In `@tests/providers/_http.test.mjs`:
- Line 113: Update the cookie extraction near getSetCookie in the test to
support Node.js 18 by using a compatible Set-Cookie fallback when
Headers.getSetCookie() is unavailable, or raise the package’s minimum supported
Node.js version to 20. Ensure CSOD bootstrap cookies are preserved on all
supported runtimes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 62525efa-a376-4d9b-87ec-c4796af34d9d
📒 Files selected for processing (4)
providers/_http.mjsproviders/csod.mjstests/providers/_http.test.mjstests/providers/csod.test.mjs
| let html; | ||
| let cookie = ''; | ||
| if (typeof ctx.fetchResponse === 'function') { | ||
| const res = await ctx.fetchResponse(cfg.homeUrl, { headers: { accept: 'text/html' } }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject bootstrap redirects.
Line 188 does not pass redirect: 'error'. fetchWithTimeout() then uses its follow default. A permitted CSOD host can redirect the bootstrap request to a private IP address. The origin validation only protects the initial URL.
Pass redirect: 'error' for the bootstrap request.
Proposed fix
-const res = await ctx.fetchResponse(cfg.homeUrl, { headers: { accept: 'text/html' } });
+const res = await ctx.fetchResponse(cfg.homeUrl, {
+ redirect: 'error',
+ headers: { accept: 'text/html' },
+});As per path instructions: “Check for command injection, path traversal, and SSRF.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const res = await ctx.fetchResponse(cfg.homeUrl, { headers: { accept: 'text/html' } }); | |
| const res = await ctx.fetchResponse(cfg.homeUrl, { | |
| redirect: 'error', | |
| headers: { accept: 'text/html' }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@providers/csod.mjs` at line 188, Update the bootstrap request in the CSOD
provider around ctx.fetchResponse to pass redirect: 'error' alongside the
existing HTML Accept header, ensuring redirects are rejected before following
any destination.
Source: Path instructions
| 'content-type': 'application/json', | ||
| accept: 'application/json', | ||
| authorization: `Bearer ${token}`, | ||
| ...(cookie ? { cookie } : {}), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not replay session cookies over HTTP.
resolveConfig() accepts http: at Line 48 and preserves that protocol in both CSOD URLs. This new header then sends bootstrap session cookies in plaintext when an entry uses an HTTP URL. Require HTTPS before the provider retrieves or replays cookies.
Proposed fix
-if (u.protocol !== 'https:' && u.protocol !== 'http:') return null;
+if (u.protocol !== 'https:') return null;As per path instructions: “Check for command injection, path traversal, and SSRF.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@providers/csod.mjs` at line 213, Update resolveConfig() and the
cookie-handling flow to require HTTPS before retrieving or replaying session
cookies. Reject configurations whose CSOD URLs use http: and ensure the cookie
header is never sent for non-HTTPS requests, while preserving existing HTTPS
behavior.
Source: Path instructions
| headers.append('set-cookie', 'tenant=kln; Secure'); | ||
| stub('{"token":"tok"}', { status: 200, headers }); | ||
| const res = await fetchResponse('https://example.com/home'); | ||
| const cookies = res.headers.getSetCookie(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -H -t f -a '^(package\.json|\.nvmrc|\.node-version|\.tool-versions)$' . \
-x sh -c 'echo "=== $1 ==="; sed -n "1,160p" "$1"' sh {}
rg -n -C 2 'node-version|setup-node|NODE_VERSION|"node"\s*:' \
package.json .github 2>/dev/null || trueRepository: santifer/career-ops
Length of output: 9680
🌐 Web query:
Node.js Headers.getSetCookie() added version official documentation
💡 Result:
The Headers.prototype.getSetCookie() method was added to the undici library—which powers the Fetch API implementation in Node.js—in version 5.19.0 [1]. Consequently, native support for getSetCookie() became available in Node.js core starting with the release of Node.js v20.0.0 [2]. This method is used to retrieve an array of all Set-Cookie header values associated with a response [3]. It is necessary because the standard Headers.get() method attempts to join multiple Set-Cookie values into a single string (often using a comma), which can incorrectly parse cookie values that contain commas (such as those in Expires attributes) [2][4]. Using getSetCookie() ensures each cookie string is correctly isolated [3][4].
Citations:
- 1: #7226 - fixes NodeJS adapter for multiple set-cookie headers (and other header issues) withastro/astro#7227
- 2: fetch response.headers.get("set-cookie") not usable to extract array of cookies nodejs/node#47755
- 3: https://developer.mozilla.org/en-US/docs/Web/API/Headers/getSetCookie
- 4:
OutgoingMessage.setHeaders(Headers)doesn't handleSet-Cookieheaders properly. nodejs/node#51599
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '=== test file context ==='
sed -n '90,125p' tests/providers/_http.test.mjs
printf '%s\n' '=== CSOD cookie feature check ==='
rg -n -C 5 'getSetCookie|set-cookie|bootstrap|cookie' providers/csod.mjs tests/providers/_http.test.mjs
printf '%s\n' '=== repository test/runtime configuration ==='
rg -n -C 3 'tests/providers|node --test|engines|node-version|setup-node' package.json package-lock.json .github 2>/dev/null || trueRepository: santifer/career-ops
Length of output: 17753
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
import re
from pathlib import Path
root = json.loads(Path("package.json").read_text())
engine = root.get("engines", {}).get("node")
source = Path("providers/csod.mjs").read_text()
test = Path("tests/providers/_http.test.mjs").read_text()
ci_versions = {}
for path in Path(".github/workflows").glob("*.yml"):
text = path.read_text()
versions = re.findall(r"node-version:\s*['\"]?([0-9]+)", text)
if versions:
ci_versions[str(path)] = versions
print(f"root engines.node = {engine!r}")
print(f"root engine includes Node 18 = {engine == '>=18'}")
print(f"test calls getSetCookie directly = {bool(re.search(r'headers\\.getSetCookie\\(\\)', test))}")
print(f"provider feature-checks getSetCookie = {bool(re.search(r'typeof res\\?\\.headers\\?\\.getSetCookie === .function.', source))}")
print(f"provider substitutes no cookies when unsupported = {\" : []\" in source or \" : []\" in source}")
print(f"CI node versions = {ci_versions}")
PYRepository: santifer/career-ops
Length of output: 460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
import re
from pathlib import Path
root = json.loads(Path("package.json").read_text())
engine = root.get("engines", {}).get("node")
source = Path("providers/csod.mjs").read_text()
test = Path("tests/providers/_http.test.mjs").read_text()
direct_call = bool(re.search(r"headers\.getSetCookie\(\)", test))
feature_check = bool(re.search(r"typeof res\?\.headers\?\.getSetCookie === ['\"]function['\"]", source))
empty_fallback = bool(re.search(r"getSetCookie\(\)\s*:\s*\[\]", source))
ci_versions = {}
for path in Path(".github/workflows").glob("*.yml"):
versions = re.findall(r"node-version:\s*['\"]?([0-9]+)", path.read_text())
if versions:
ci_versions[str(path)] = versions
print(f"root engines.node = {engine!r}")
print(f"root engine includes Node 18 = {engine == '>=18'}")
print(f"test calls getSetCookie directly = {direct_call}")
print(f"provider feature-checks getSetCookie = {feature_check}")
print(f"provider uses an empty-array fallback = {empty_fallback}")
print(f"CI node versions = {ci_versions}")
PYRepository: santifer/career-ops
Length of output: 558
Support Node.js 20 or add a Set-Cookie fallback.
The package supports Node.js >=18, but the test calls Headers.getSetCookie() directly and CI only tests Node.js 24. On Node.js 18, CSOD bootstrap cookies are omitted, which can cause HTTP 401 CSOD Unauthorized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/providers/_http.test.mjs` at line 113, Update the cookie extraction
near getSetCookie in the test to support Node.js 18 by using a compatible
Set-Cookie fallback when Headers.getSetCookie() is unavailable, or raise the
package’s minimum supported Node.js version to 20. Ensure CSOD bootstrap cookies
are preserved on all supported runtimes.
1 participant
Closes #2768
Two coupled bugs: CSOD tenants that enforce session cookies scan as
401/ zero jobs, and the helper that would fix it —fetchResponse()— has never worked.1.
csod.mjs— replay the bootstrap session cookiesCSOD's
/services/x/career-site/v1/searchneeds the cookies the bootstrap home page sets alongside the anonymous JWT. The provider read that page withctx.fetchText(), which discards response headers, soSet-Cookiewas dropped and every search arrived unauthenticated.The bootstrap now goes through
ctx.fetchResponsesoSet-Cookieis visible, and the cookies are replayed as acookieheader on each search POST.cookieHeaderFrom()— keeps the leadingname=value, strips attributes (path/HttpOnly/Secure/SameSite/Expires) since those describe browser-jar storage rules and mean nothing on a request, last-wins per name.cookieheader, not an empty one. This matters:career-ohbworks today without cookies and must keep working.ctx.fetchTextwhen a ctx has nofetchResponse, so older embedders and existing test mocks are unaffected.cfg.homeUrlandcfg.searchApiare built from the same parsed origin, so the replayed cookies cannot reach a third-party host.The header comment claiming "no login, no session cookies needed" is corrected — that assumption is what the bug was made of.
2.
_http.mjs— makefetchResponse()actually runfetchWithTimeoutends inreturn await consume(res), so this threwTypeError: consume is not a functionon every call. It went unnoticed because it has no callers —git grepmatches only_http.mjsand_types.js. (Its comment cites astartup.chprovider as precedent; that provider isn't in this repo.)Why not just
(res) => res: returning the liveResponsewould reintroduce the failure documented directly above it in the same file — a server that sends headers then stalls its body hangs the caller with the abort timer already cleared, which that comment records as having "froze full-directory sweeps silently". So the body is read inside the timer window and returned as an equivalentResponse. Callers keep the standard{ headers, status, text() }shape without inheriting the stall.Null-body statuses (204/205/304) are guarded — the
Responseconstructor throws on those with a body. RepeatedSet-Cookiesurvives the reconstruction, which is the entire point of the helper and is now pinned by a test.Since the function never successfully executed, there's no prior behaviour to preserve.
Verification
Live, against the public
careers-kln.csod.comtenant (siteId 14):34 matches the
totalCountreturned by a hand-issued request with a cookie jar, so the board is fully enumerated, not partially.Tests added:
tests/providers/csod.test.mjs— cookie replay with attributes stripped; header omitted when the tenant sets none;fetchTextfallback whenctxhas nofetchResponse.tests/providers/_http.test.mjs—fetchResponse()preserves repeatedSet-Cookie, body stays readable, 204 doesn't throw. These fail onmainwithconsume is not a function.Suite status on this branch: provider suite 1,410 passed / 0 failed across 79 files;
tests/providers/_http.test.mjsandtests/providers/csod.test.mjsgreen. (test-all.mjsOOMs on my ARM64 box, so files were run individually.)Both tests were confirmed red before the fix.
Summary by CodeRabbit