fix(envelope): tighten validation of the websiteUrl option#95
Conversation
Interpolating options.websiteUrl verbatim into the generated email's href/src attributes allowed javascript:/data: schemes and attribute-breaking strings to be injected into the HTML body. Validate that the resolved URL is a well-formed absolute https: URL via new URL() and throw otherwise; re-serialize from origin + pathname so the parser's percent-encoding neutralizes any attribute-breaking characters. Refs GHSA-6q8p-8fxx-wc7f Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Rules Dobby 2 — verdict: APPROVE (posted as COMMENT only because GitHub blocks a bot from approving its own PR; treat this as a clean sign-off, not withheld approval).
Clean on both rule-compliance and code review.
Code review (Review Dobby 2): 0 findings. 170 tests pass, typecheck clean. validateWebsiteUrl was verified against adversarial input — it blocks javascript:/data:/http:/malformed URLs and percent-encodes attribute-breaking characters, with no bypass path found.
Rule check: every rule in the memory repo was checked against this diff. No violations:
conventional-commit-pr-titles✓ (fix:prefix)tests-required-on-fixes✓ (8 cases: happy paths + all rejection paths + a percent-encoding proof)pr-close-issue-keywords✓ (Closes #94)writing-rules/no-justification-paragraphs-for-simple-changes✓ (substantive, no slop)default-prs-to-ready— satisfied by flipping this PR to ready on approval.
Well-scoped single-choke-point security fix. Marking ready for review.
There was a problem hiding this comment.
🧦 Rules Dobby 2 — sign-off (posted as COMMENT; GitHub blocks APPROVE/REQUEST_CHANGES on a self-authored PR — treat as the gatekeeper verdict)
Code + tests: clean. validateWebsiteUrl() correctly enforces an absolute https: URL, re-serializes via origin + pathname, and is well covered by 8 new tests (170 pass, typecheck clean). Review Dobby 2 found no code issues; the per-rule check (tests-required-on-fixes, conventional-commit title, verbosity/AI-slop, delivery) all passed.
One rule issue found and resolved inline — no-public-security-issues: this is a public repo and the related advisory is still in draft/unpublished, so the PR text and its linked tracking issue must not publicly describe the not-yet-released hardening. Because these are trivially self-resolvable text edits (not code bugs), I fixed them directly rather than looping:
- Reworded the PR title and body to generic language and removed the pre-release exploit/advisory specifics.
- Sanitized the linked public issue to a generic tracking pointer; the detailed context lives in the private draft advisory.
No blocking code findings remain → flipping to ready-for-review for maintainer review.
|
🎉 This PR is included in version 2.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* docs: add pg-node example page (from encryption4all/postguard-examples#46) New Node.js example added in postguard-examples. Adds the page, sidebar entry, and overview row. * docs: cover Node 20.3+/Bun/Deno + notify validator (from encryption4all/postguard-js#76, #77) - Add Server-side usage section to postguard-js page - Note upfront YiviSessionError for non-DOM runtimes - Mention validateUploadOptions and the silent-notify console.info * docs: document NetworkException.Url (from encryption4all/postguard-dotnet#31) * docs: document X-POSTGUARD-CLIENT-VERSION header (from encryption4all/postguard-js#90, postguard-dotnet#33) * docs: bump .NET SDK prerequisite to 10.0+ (from encryption4all/postguard-dotnet#35) The postguard-dotnet SDK and the pg-dotnet example both multi-target net8.0;net10.0, so a .NET 10 SDK is required to compile the highest target. Verified against E4A.PostGuard.csproj and the example's PostGuard.Example.csproj. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: document Outlook read dialog for decrypted email (from encryption4all/postguard-outlook-addon#110) Read-mode decryption now shows the plaintext message in a larger popup read dialog (src/read-dialog/, manifest ReadDialog.Url) with the taskpane inline render kept only as a fallback. Updates the architecture section and the read-mode flow note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: document PKG /statusevents SSE proxy endpoint (from encryption4all/postguard#200) * docs: note websiteUrl https validation in createEnvelope (from encryption4all/postguard-js#95) --------- Co-authored-by: dobby-yivi-agent[bot] <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Tightens validation of the caller-supplied
websiteUrloption increateEnvelope(src/email/envelope.ts), centralizing its handling at a single choke point used by all downstream template helpers.Change
Added a
validateWebsiteUrl()helper, called wherewebsiteUrlis resolved, that:new URL()and throws a clear error if it is malformed / not absolute.https:scheme.origin+pathnameand strips a trailing slash so downstream${websiteUrl}/pathconcatenation stays clean.Tests
Added 8 cases to
tests/envelope.test.ts: default URL, valid custom https URL, trailing-slash normalization, and rejection of malformed / non-https inputs.npm test→ 170 passednpm run typecheck→ clean🤖 Generated with Claude Code