Sign-off: security-review + external-maintainer-readiness (branch sec-review/maintainer-readiness)
- Dominant language
- TypeScript
- Stars
- 0
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
🤖 PSS agent — @jeswr's agent for \`prod-solid-server\` / the Solid app+Pod-Manager suite
Branch \`sec-review/maintainer-readiness\` (HEAD \`ecc3d22\`) is a security-review + external-maintainer-readiness pass on \`@jeswr/auth-solid\` (bead suite-tracker-68b0). It **changes security behaviour** (strictly hardening), so it stages here for your sign-off rather than direct-landing.
## Security findings fixed (all with regression tests)
**HIGH — ID-token JWS signature was never verified.** In the installed \`@auth/core\` 0.41 flow, \`processAuthorizationCodeResponse\` validates alg/claim-presence/iss/aud/time but **not** the JWS signature (that lives only in oauth4webapi's separate \`validateApplicationLevelSignature\`, which @auth/core never calls). So \`profile.webid\` was trusted from a signature-unverified token. Fix: new \`src/idTokenVerify.ts\` verifies the raw id_token against the OP JWKS (jose \`jwtVerify\`, issuer+audience pinned, rejects HS*/none pre-JWKS, fail-closed); WebID now read from the signature-verified payload. Tests: correctly-signed accepted; wrong-key / tampered / HS256 / no-id_token / wrong-aud / wrong-iss / sub-mismatch all rejected.
**HIGH — discovery/JWKS fetch followed redirects.** An https metadata/JWKS endpoint that 302s to http (or a foreign origin) would be followed inside fetch, allowing plaintext/cross-origin JWKS key substitution that defeats the signature boundary above. Fix: \`fetchNoRedirect\` (redirect:manual + reject 3xx/opaque-redirect, fail-closed) on both discovery and JWKS, layered on the existing https-only transport guard. Tests: discovery/JWKS 302 rejected, opaque-redirect rejected, happy path still verifies.
**MEDIUM — every customFetch leg now https-enforced** (was token-leg only; @auth/core sets oauth4webapi \`allowInsecureRequests\` whenever a customFetch exists, so this package is the *only* https enforcement — an http userinfo_endpoint would have received the access token in clear).
**MEDIUM — empty \`DPoP-Nonce\` header no longer triggers a spurious retry** (Headers.get returns "" not null; the gate now treats falsy as absent).
**MEDIUM — api-extractor pinned** as a devDependency (was \`npx --yes @microsoft/api-extractor@7\` = non-reproducible / network-dependent gate).
## Verification
- roborev PASS on HEAD (single daemon, serialized); all prior-round findings walked back and confirmed gone from HEAD (not just a diff-scoped later PASS).
- I re-ran the security suite myself on HEAD: **102 tests pass** including the 9 signature-verification cases and the 4 redirect-refusal cases.
- Clean-room \`GIT_SSH_COMMAND=false npm ci --ignore-scripts\` OK; dist rebuilt + committed; api-extractor snapshot byte-identical (all new code is internal); check:dist + api:check green; #78 lockfile clean.
- One behaviour change to note for consumers: the \`profile\` callback is now **async** (it awaits JWKS verification).
## Follow-ups (beaded, non-blocking)
- Refresh-grant helper (persists refreshToken/expiresAt but ships none; compose with @jeswr/solid-dpop).
- Wire @jeswr/guarded-fetch into the \`Solid({ fetch })\` seam (guarded-fetch phase-2) — would consolidate the redirect-refusal helper.
- Suite-wide \`npm ci\`-broken-lockfile sweep (npm-11 drops @emnapi wasm optionals) — a general finding, not specific to this repo.
Merge when you're happy with the security review.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the sec-review/maintainer-readiness branch at HEAD ecc3d22 and read src/idTokenVerify.ts, then run the security suite covering signature verification and redirect refusal. The work is presented as completed and awaiting sign-off, with verification already reported, so there is no unclaimed implementation path for a newcomer.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authentication, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 15/100