Repository navigation
Conversation
Executes the XMLDSig XSLT transform (http://www.w3.org/TR/1999/REC-xslt-19991116) behind a new non-default `xslt` feature, with the XSLT engine compiled into xml-sec. - Engine sources live under src/xslt with tests in sibling files; the standalone engine crate builds the same sources. Entry points report what they consumed whether they succeed or fail; bounded parsing charges the retained tree and every parser pass; a caller work ceiling bounds every kind of execution work together. - Transform::Xslt requires an xsl:stylesheet or xsl:transform element as the sole content of its Transform and compiles it from the parsed tree, with entity and character references replaced. The signature builder writes a parsed stylesheet with what it inherited, so it does not depend on its original document. Node-set input goes through implicit inclusive C14N 1.0. - Every XSLT execution of an operation shares one policy allowance for work, output and recursion depth, failed executions included. Compilation, the parsed input and execution share one workspace; ResourcePolicy gains max_xslt_work, max_xslt_workspace_bytes, max_xslt_output_bytes and max_xslt_recursion_depth, and stylesheet and input text obey the XML document, node, depth, namespace-binding and parse-work limits. Inside a query, the input is decoded into the query's account before the workspace is reserved, and the query's remaining work bounds every parser pass and the execution. - TransformPolicy::xslt_stylesheet_semantics selects how the stylesheet is read: Specification (default) gives it every namespace declaration in scope at its Transform and an ancestor's xml:space="preserve"; Libxmlsec1 compiles the content alone and skips unbound prefix-list entries, as libxmlsec1 and libxslt do. xml-sec-config parses it. - Stylesheets resolve no external resources. - An octet-consuming transform after Enveloped (XSLT, Base64) no longer reports a dependency cycle through the excluded SignatureValue. - xml-sec-cli gains an xslt feature; the capability ledger classifies xmlSecHrefXslt as feature-gated and policy-permitted. - The structured crates move to 0.8 (structured-asn1 0.8.1 for the remaining-work query); xml-sec-xades checks certificate paths through structured-ltv; VerifiedSession::begin_digest provides the incremental digest their shared contract needs. - CI builds the xmlsec1 oracle with libxslt and runs tests and clippy with the xslt feature; docs describe the transform, its limits and the stylesheet setting. BREAKING CHANGE: Transform is #[non_exhaustive]; ResourcePolicy and TransformPolicy have new public fields; VerifiedSession::digest_in is replaced by begin_digest. Closes #247
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 7f436bc8a9
ℹ️ 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".
 [High impact] The reviewed changes are safe to merge; no blocking findings remain.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- an XPath evaluation admits each unit of work against the execution's work ceiling before doing it, so it stops where the ceiling refuses it instead of scanning first and failing the aggregate charge afterwards - a bounded parse charges the xml:id and DTD ID index (keys, table and the list of owners) to the tree before allocating it, and counts it in the retained bytes the adapter subtracts from the workspace - the markup and serialization a parsed stylesheet carries count against the XSLT workspace for the whole transform - a node-set given to XSLT is converted with inclusive C14N 1.0, so that algorithm must be on the transform allowlist for signing and verification even when the chain ends with octets - namespace declarations a DTD defaults on stylesheet elements are kept when the stylesheet is serialized from its tree Regression tests cover each case: an XPath scan under a small ceiling, ID documents retaining more than equal-sized plain ones, a parsed stylesheet refused where its given markup fits, an allowlist without inclusive C14N, and a DTD-defaulted prefix.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 44 seconds. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 61.90% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 504 functions across 55 files. (5 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely identifies the main change: executing the XSLT transform in XMLDSig. The feat(xmldsig)!prefix also signals a feature and breaking change.Linked Issues check Issue [ #247] coding requirements are addressed. The feature-gatedTransform::Xsltadapter uses the shared native engine. It supports stylesheet parsing, inclusive C14N for node-set input, chained an…Out of Scope Changes check The changes stay within [ #247]. Embedded engine packaging, policy and budget accounting, configuration, capability ledgers, interoperability setup, and tests support the XSLT transform. The structured…Full details: Linked Issues check
-
Explanation
Issue [
#247] coding requirements are addressed. The feature-gatedTransform::Xsltadapter uses the shared native engine. It supports stylesheet parsing, inclusive C14N for node-set input, chained and terminal output, operation-wide resource limits, resolver denial, typed errors, policy control, signing, verification, replay, and capability reporting. The integration, corpus, and xmlsec1 interoperability tests cover the required transform paths and feature configurations.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within [
#247]. Embedded engine packaging, policy and budget accounting, configuration, capability ledgers, interoperability setup, and tests support the XSLT transform. The structured-crate, XAdES, DSA, digest, and cipher-reference changes support shared resource, parsing, and digest mechanisms used by the operation. No unrelated change is demonstrated.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 61.90% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 504 functions across 55 files. (5 skipped: 5 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cef22a5c2b
ℹ️ 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".
- a bounded parse charges the unparsed entities a DTD declares (their names, URIs and the table holding them) to the tree before inserting them, so the retained bytes the adapter subtracts from the workspace include them; one helper now meters both retained indexes - a stylesheet serialized from its <Transform> pays the query for each node before writing it: the start-tag scan, the comparison of its in-scope namespace bindings with its parent's, and its names, values and text, instead of paying only for the resulting text afterwards - the scan for a document type declaration and the policy preflight of the input are paid separately, each before it runs Regression tests cover each case: a DTD of NDATA entities retaining more than one of parsed external entities, a stylesheet whose namespace scans exceed the query's remaining work, and a large prolog with work for only one and a half input scans.
There was a problem hiding this comment.
💡 Codex Review
Line 2578 in ab3855a
Charge attribute-set validation to the compiler workspace
When an untrusted stylesheet declares many attribute sets and use-attribute-sets edges, this validation allocates an unmetered declaration HashSet, followed by the cycle check's index map, adjacency vectors, incoming-count array, and ready queue, while the already-compiled stylesheet remains live. Those temporary allocations are never passed through CompilerState::charge_owned, so a stylesheet whose retained representation nearly fills max_xslt_workspace_bytes can exceed that limit by the size of the validation graph and still compile successfully. Reserve and release this validation workspace through the compiler's existing owned-byte meter.
AGENTS.md reference: AGENTS.md:L41-L44
ℹ️ 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".
- Share stylesheet capture and execution allowances across the operation. - Fund query serialization and XML preflight before reserving native storage. - Preserve repeated namespace declarations around entity-expanded children. - Bound entity materialization and attribute-set validation workspace. - Admit digest finalization before entering the provider and use inline output. - Cover refusal ordering, buffer growth peaks and namespace preservation. Refs #247
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d42b4d908
ℹ️ 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".
Use published structured crates 0.9.1 throughout the workspace. Bound transcoding before allocation and retain decoded capacity in the shared workspace. Admit serialized output against the current query work ceiling while keeping its output quota separate. Preserve stylesheet document comments and processing instructions in both execution paths. Add regressions for all three cases and include serialization in the exact aggregate-work boundary test. Avoid an empty workspace guard for borrowed UTF-8 input.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d12930c04f
ℹ️ 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".
Link descriptor transform parsing to the reference context's XSLT allowance, including nested key resources. A public decryption regression checks the exact combined capture/execution boundary against direct reference resolution.
Preserve comments and processing instructions while omitting whitespace-only stylesheet siblings. Charge their validation before scanning and cover signing, verification and replay with carriage-reference regressions.
Read DSA public components through the shared certificate execution before backend validation, preserving resource refusals and borrowed magnitudes. Keep RSA/EC return values small without boxing and cover the session-work boundary. Document standalone stylesheet whitespace handling.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bcea86053
ℹ️ 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".
Admit native output capacity, its exact accounted copy and retained stylesheet storage under the XSLT workspace limit before copying. Keep query memory accounting independent. Add an amplification regression for below/exact peak boundaries and released charges on both outcomes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 299b217008
ℹ️ 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".
| stylesheet_bytes = stylesheet_bytes.saturating_add(stylesheet.markup_len()); | ||
| policy | ||
| .resources | ||
| .validate_xml_document_len(stylesheet_bytes)?; |
There was a problem hiding this comment.
When a caller creates a large XsltStylesheet and then calls build_template_with_policy with a smaller max_xslt_workspace_bytes, this validation checks the stylesheet only against max_xml_document_bytes, so it returns a template that the subsequent signing operation under the same policy must reject while capturing the generated <Transform>. Validate the generated stylesheet against the target policy's XSLT workspace before allocating and returning an unusable template.
AGENTS.md reference: AGENTS.md:L32-L34
Useful? React with 👍 / 👎.
| let projected = push_metered( | ||
| document, | ||
| parent, | ||
| NodeKind::Element { |
There was a problem hiding this comment.
When an XSLT input contains one element with many ordinary or DTD-defaulted attributes and max_xslt_workspace_bytes is smaller than the XML document limit, raw_attributes, the collected attributes vector, and the duplicate-name HashSet are all allocated before this first owned-byte charge. The parser can therefore hold several attacker-sized attribute buffers before rejecting the element, exceeding the configured workspace; reserve and meter those temporary containers as they grow rather than charging only after constructing the final NodeKind.
AGENTS.md reference: AGENTS.md:L41-L44
Useful? React with 👍 / 👎.
| transform | ||
| .document() | ||
| .input_text() | ||
| .get(first.range().start..last.range().end) |
There was a problem hiding this comment.
When internal DTDs are allowed and an entity supplies the stylesheet element after a literal processing instruction or comment, the literal sibling's range points into the document body while the expanded element's range can point backward into the DTD, making this range reversed and causing get to return None; parse_xslt_transform then reports a missing stylesheet. XML 1.0 §4.4 defines entity replacement as parsed document content, so derive this markup from the already-expanded child tree rather than assuming child source ranges are ordered.
AGENTS.md reference: AGENTS.md:L83-L90
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Summary
xsltfeature, using the native XSLT 1.0 engine embedded inxml-sec.TransformPolicy::xslt_stylesheet_semantics, with specification semantics as the default.Changes
xsl:stylesheetorxsl:transformparameter. Stylesheets compile from their parsed tree, preserving replaced entity/character references and the selected inheritance semantics. Node-set input uses implicit inclusive C14N 1.0.structured-ltvfor path checks and session-backed incremental digests with caller-owned output and current-query finalization admission.Envelopedno longer introduce a dependency cycle through the excludedSignatureValue. CI builds the xmlsec1 oracle with libxslt and exercises XSLT-enabled configurations.Testing
Workspace default, XSLT/all-XML-backend and all-feature nextest suites, SoftHSM, doctests, clippy, formatting and rustdoc. XMLsec1/libxslt interoperability covers public signing/verification and engine output; alloc-only shared mechanisms and actual WASM C14N execution are checked separately.
Replaces #252.
BREAKING CHANGE: Transform is #[non_exhaustive]; ResourcePolicy and TransformPolicy have new public fields; VerifiedSession::digest_in is replaced by begin_digest.
Closes #247
Summary by CodeRabbit
New Features
Bug Fixes
Documentation