feat(security): default 30 s render timeout for UntrustedHtml - #380
Merged
Merged
Conversation
Add SecurityPolicy.RenderTimeout, a default hard cap on conversion time for renders that use the policy. UntrustedHtml now defaults to 30 seconds, so a hostile document cannot hold a worker indefinitely when the caller sets no timeout. SafeDefault and TrustedTemplate stay uncapped. HtmlPdfOptions.Timeout stays the per-render knob and always wins over the policy default. Timeout.InfiniteTimeSpan now means "no cap" (it previously cancelled immediately, like any negative value). The TimeoutException message names the setting that fired. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unrelated pipeline cancellations can be incorrectly reported as policy timeout failures.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds a default 30-second render timeout for untrusted HTML while preserving explicit timeout overrides.
Changes:
- Adds
SecurityPolicy.RenderTimeoutand configuresUntrustedHtml. - Resolves policy and per-render timeout precedence.
- Updates tests, API baseline, and security documentation.
| File | Description |
|---|---|
src/NetPdf/HtmlPdf.cs |
Applies effective render timeout and reports its source. |
src/NetPdf/HtmlPdfOptions.cs |
Documents timeout precedence and infinite opt-out. |
src/NetPdf/Resources/SecurityPolicy.cs |
Adds the policy timeout and 30-second untrusted default. |
tests/NetPdf.UnitTests/HtmlPdfFacadeTests.cs |
Tests defaults, overrides, messages, and opt-out behavior. |
tests/NetPdf.UnitTests/Api/PublicApi/NetPdf.txt |
Updates the public API baseline. |
README.md |
Documents the untrusted-render timeout. |
docs/security/deployment.md |
Updates deployment guidance and checklist. |
CHANGELOG.md |
Records the new security feature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4144a1ef7
ℹ️ 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".
…Save PR #380 review (Copilot + Codex): - Translate an OperationCanceledException into TimeoutException only when the render timer itself fired. A stage that cancels for its own reason (e.g. a custom IResourceLoader) keeps its original exception. - PdfDocument.Save takes no token, so the pipeline re-checks cancellation after serialization. A deadline that passes during Save now fails the render instead of returning a PDF past the cap. Tests: unrelated loader cancellation propagates unchanged (fails without the fix); a token-ignoring stage that crosses the deadline still times out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
SecurityPolicy.RenderTimeout(TimeSpan?, defaultnull= no cap).SecurityPolicy.UntrustedHtmlnow sets it to 30 seconds.HtmlPdfOptions.Timeoutstays the optional per-render knob and always wins:null(default) → use the policy'sRenderTimeout.Timeout.InfiniteTimeSpan→ no cap, even underUntrustedHtml.TimeSpan.Zero/ other negative → fail immediately (unchanged).TimeoutExceptionmessage names the source (HtmlPdfOptions.TimeoutorSecurityPolicy.RenderTimeout).Why
Follow-up from the security review:
UntrustedHtmllocked down every fetch surface and budget, but a render had no time bound unless the caller remembered to setTimeout. A pathological document could hold a worker indefinitely.Compatibility
SafeDefault/TrustedTemplatebehavior is unchanged.UntrustedHtmlrenders longer than 30 s now throwTimeoutExceptionunlessTimeoutis set.Timeout = Timeout.InfiniteTimeSpanpreviously cancelled immediately (treated as negative); it now means no cap, matching .NET convention.Tests
9 new facade tests (policy defaults, policy cap fires, explicit override, infinite opt-out, error source, UntrustedHtml ordinary render). Public API baseline updated. Unit suite: 8658 passed / 3 skipped.
Docs: README security checklist,
docs/security/deployment.md, CHANGELOG[Unreleased].🤖 Generated with Claude Code