Skip to content

Harden URL reads and XML serialization#343

Merged
vinitkumar merged 10 commits into
masterfrom
agent/harden-untrusted-input
Jul 19, 2026
Merged

Harden URL reads and XML serialization#343
vinitkumar merged 10 commits into
masterfrom
agent/harden-untrusted-input

Conversation

@vinitkumar

@vinitkumar vinitkumar commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • bound decoded URL responses to 10 MiB by default, reject redirects, credentials, unsupported schemes, and non-public destinations, with an explicit trusted private-network opt-in
  • prevent XML injection through namespace and custom type attributes, and reject XML 1.0-forbidden characters in both Python and Rust serializers
  • disable outdated Rust accelerators that do not enforce the XML character boundary
  • sync the stale urllib3 requirements pin to 2.7.0 and add weekly Python Dependabot coverage for the root and docs manifests
  • document the secure URL-reader defaults and update the lat.md architecture/test graph

Compatibility note

readfromurl() now rejects private destinations by default. Trusted callers that intentionally read a private endpoint must pass allow_private_networks=True; they can also set max_response_bytes explicitly.

Validation

  • 460 Python tests; 100% statement coverage
  • 48 Rust tests
  • Ruff: pass
  • ty: pass
  • Cargo fmt: pass
  • Cargo Clippy with all targets/features and warnings denied: pass
  • dependency audit for requirements.txt and requirements.in: no known vulnerabilities
  • lat check: pass

Summary by Sourcery

Harden remote JSON reading and XML serialization by enforcing public-only, bounded URL requests, validating XML 1.0 character and namespace safety across Python and Rust backends, and updating dependencies, documentation, and tests accordingly.

New Features:

  • Add configurable size limits and private-network opt-in to the URL JSON reader to bound decoded responses and restrict remote targets by default.

Bug Fixes:

  • Ensure XML serialization rejects invalid XML 1.0 characters in both Python and Rust paths, preventing malformed output and wrapping parser failures as InvalidDataError.

Enhancements:

  • Harden URL reading by validating schemes, credentials, host resolution, HTTP status, and Content-Length before decoding JSON.
  • Treat XML namespace metadata and custom type attributes as properly escaped attribute values, validating namespace prefixes and xsi mappings for safer output.
  • Gate Rust accelerator usage on enforcing XML character rules so outdated backends are disabled in favor of the secure Python serializer.

Build:

  • Update the urllib3 dependency pin to 2.7.0 and expand Dependabot configuration to monitor root and docs Python manifests weekly.

Documentation:

  • Document URL reader security defaults, XML output safety guarantees, and Rust backend selection behavior in the architecture and test behavior guides.

Tests:

  • Extend Python and Rust test suites to cover URL reader security and size limits, XML 1.0 character validation, namespace handling, Rust backend eligibility, and pretty-print error wrapping.

@sourcery-ai

sourcery-ai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR hardens remote URL reading and XML serialization by enforcing public-only, size-bounded HTTP(S) reads, adding XML 1.0 character validation and secure namespace/type handling in both Python and Rust paths, gating Rust accelerators behind the same safety checks, and tightening dependency and documentation coverage around these behaviors.

Sequence diagram for hardened readfromurl URL loading

sequenceDiagram
    actor Caller
    participant Utils as readfromurl
    participant Validator as _validate_url
    participant Client as _get_http_client
    participant HTTP as urllib3_HTTPClient

    Caller->>Utils: readfromurl(url, params, max_response_bytes, allow_private_networks)
    Utils->>Utils: validate max_response_bytes
    Utils->>Validator: _validate_url(url, allow_private_networks)
    Validator-->>Utils: ok or URLReadError
    Utils->>Client: _get_http_client()
    Client-->>Utils: http, timeout
    Utils->>HTTP: request("GET", url, fields=params, timeout, retries=False, redirect=False, preload_content=False)
    HTTP-->>Utils: response
    Utils->>Utils: check status, Content-Length, size limits
    Utils->>HTTP: read(max_response_bytes + 1, decode_content=True)
    HTTP-->>Utils: response_data
    Utils->>Utils: json.loads(response_data.decode("utf-8"))
    Utils-->>Caller: JSONValue or URLReadError
Loading

File-Level Changes

Change Details Files
URL reader now enforces public, credential-free HTTP(S) with bounded decoded responses and clearer error handling.
  • Introduce _validate_url helper to parse URLs, enforce http/https, forbid credentials, and require hostnames.
  • Resolve hostnames to IPs and reject non-global/private/link-local addresses by default, with an allow_private_networks opt-in parameter.
  • Add max_response_bytes parameter with validation, defaulting to 10 MiB, and use preload_content=False plus read() to cap decoded response size.
  • Validate Content-Length headers against the configured limit and reject invalid or oversized values.
  • Disable redirects at the urllib3 layer and ensure non-200 status codes, network failures, decoding errors, and JSON parse errors map to URLReadError with updated messages.
  • Extend tests to cover private-network rejection, DNS resolution behavior, unsupported schemes/credentials, invalid limits/content lengths, and response size bounding, and update lat behavior/tests docs.
json2xml/utils.py
tests/test_utils.py
lat.md/behavior.md
lat.md/tests.md
XML escaping, CDATA, namespaces, and custom type attributes now share XML 1.0 character validation and attribute-safe escaping.
  • Add a regex-based XML 1.0 invalid-character detector and _validate_xml_chars helper.
  • Update escape_xml to validate characters first, then apply escaping, and use it for custom type attributes via make_attrstring.
  • Update wrap_cdata to validate characters before wrapping and ensure CDATA end markers are safely split.
  • Introduce _NamespaceFormatter validation for namespace prefixes (type, name, xml* rules) and route namespace and xsi values through escape_xml.
  • Add tests for namespace value escaping, invalid prefixes, malformed xsi mappings, custom type attribute escaping, and rejection/preservation of forbidden vs allowed whitespace characters, and document XML output safety in lat behavior/architecture docs.
json2xml/dicttoxml.py
lat.md/behavior.md
lat.md/architecture.md
tests/test_dict2xml.py
Rust-backed XML helpers are aligned with Python XML 1.0 rules, and outdated accelerators are disabled at import time.
  • Implement invalid_xml_char and validate_xml_chars in Rust and use them from write_escaped_text, write_escaped_attr, write_cdata, escape_xml_py, and wrap_cdata_py to reject forbidden characters.
  • Change escape_xml_py and wrap_cdata_py to return PyResult and propagate ValueError on invalid input.
  • Add tests covering forbidden-character rejection in Rust escape, CDATA, and main dicttoxml paths.
  • Add a _rejects_invalid_xml probe in dicttoxml_fast that calls the Rust escape helper with a forbidden character and enables the Rust backend only when it raises ValueError, logging a warning when it does not.
  • Extend fast-fallback tests to assert the backend selector respects this safety gate.
rust/src/lib.rs
json2xml/dicttoxml_fast.py
tests/test_rust_dicttoxml.py
tests/test_dicttoxml_fast_fallback.py
lat.md/tests.md
lat.md/architecture.md
Json2xml pretty-printing now preserves InvalidDataError semantics for both serializer and parser failures.
  • Wrap dicttoxml.dicttoxml calls in Json2xml.to_xml with a try/except that re-raises ValueError as InvalidDataError.
  • Add a test that simulates defusedxml.minidom.parseString raising ExpatError and asserts Json2xml.to_xml surfaces InvalidDataError.
  • Clarify behavior in lat behavior docs around pretty-print reparsing as a validation step.
json2xml/json2xml.py
tests/test_json2xml.py
lat.md/behavior.md
Python dependency hygiene is improved via updated urllib3 pinning and expanded Dependabot coverage.
  • Bump urllib3 requirement from 2.5.0 to 2.7.0 in requirements.txt to match resolver inputs.
  • Extend Dependabot configuration to monitor pip dependencies in the root and docs directories on a weekly schedule.
  • Update architecture docs to note Dependabot coverage for Python dependency manifests.
requirements.txt
.github/dependabot.yml
lat.md/architecture.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (32038e6) to head (bf27193).

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #343   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          759       833   +74     
=========================================
+ Hits           759       833   +74     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Comment thread json2xml/dicttoxml.py Fixed

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Fixed security issues:

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/test_dict2xml.py" line_range="92-94" />
<code_context>

         assert result == f'<key {attr_name}="value">payload</key>'.encode()

+    # @lat: [[tests#Conversion behavior#Custom type attributes use shared escaping]]
+    def test_custom_type_attribute_value_is_escaped(self) -> None:
+        """A caller-provided type attribute must not bypass value escaping."""
+        result = dicttoxml.dicttoxml(
+            {
+                "key": {
+                    "@attrs": {"type": 'safe" injected="yes'},
+                    "@val": "payload",
+                }
+            },
+            root=False,
+            attr_type=False,
+        )
+
+        assert result == (
+            b'<key type="safe&quot; injected=&quot;yes">payload</key>'
+        )
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test that invalid XML 1.0 characters in attribute values are rejected

The new tests cover XML 1.0–forbidden characters in element text and CDATA and verify escaping for custom `type` attributes, but they don’t directly assert that XML 1.0 validation applies to attribute values. Please add a test like:

```python
with pytest.raises(ValueError, match="not allowed in XML 1.0"):
    dicttoxml.dicttoxml(
        {
            "key": {
                "@attrs": {"type": "before\x00after"},
                "@val": "payload",
            }
        },
        root=False,
        attr_type=False,
    )
```

This ensures attributes cannot bypass the forbidden-character checks.

```suggestion
    # @lat: [[tests#Conversion behavior#Custom type attributes are validated]]
    def test_custom_type_attribute_value_forbidden_chars_rejected(self) -> None:
        """XML 1.0–forbidden characters in caller-provided type attributes must be rejected."""
        with pytest.raises(ValueError, match="not allowed in XML 1.0"):
            dicttoxml.dicttoxml(
                {
                    "key": {
                        "@attrs": {"type": "before\x00after"},
                        "@val": "payload",
                    }
                },
                root=False,
                attr_type=False,
            )

    # @lat: [[tests#Conversion behavior#Namespace metadata cannot inject attributes]]
    def test_namespace_values_are_escaped_as_xml_attributes(self) -> None:
        """Namespace values must not be able to inject sibling attributes."""
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread tests/test_dict2xml.py
@vinitkumar
vinitkumar merged commit 232daef into master Jul 19, 2026
65 checks passed
@vinitkumar
vinitkumar deleted the agent/harden-untrusted-input branch July 19, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants