[api][runtime][java][python] Require HTTPS and add digest pinning for URL skill sources

[rob-9]

Purpose of change

Closes #1003.

Remote skill archives are trusted instructions and code: their SKILL.md content is consumed by the agent, and bundled scripts can become executable through the built-in tools. Until now, Skills.fromUrl(...) / Skills.from_url(...) accepted plain HTTP and extracted whatever arrived, without verifying an operator-provided integrity expectation.

This change makes URL skill sources secure by default and adds optional integrity verification across the Java, Python, and YAML surfaces.

• Reject plain HTTP URL skill sources by default; HTTPS is required.
• Keep plain HTTP available only through explicitly named unsafe factory methods or the allow_insecure_http YAML option.
• Let operators pin an expected SHA-256 digest, which is verified against the downloaded archive before extraction.
• Enforce the transport and digest rules again at runtime, so sources arriving through plan serialization or YAML cannot bypass API-level validation.
• Fail with a clear error on unsupported cross-protocol redirects and clean up temporary archives on every failure path.
• Add a structured YAML url_sources list while preserving the existing urls shorthand for unpinned HTTPS sources.
• Keep Java and Python aligned on configuration, security behavior, tests, documentation, and generated YAML schemas.

Existing direct and same-protocol-redirecting HTTPS sources are unaffected. Existing plain HTTP sources must now opt in explicitly.

Tests

• Targeted Java API tests: 31 passed.
• Targeted Java runtime tests: 38 passed.
• Python tests covering the changed API, YAML package, and runtime skill paths: 151 passed.
• Java/Python agent-plan cross-language tests: 9 passed, 1 skipped.
• Java Spotless check passed.
• Ruff passed on the changed code; the six remaining reported violations are pre-existing and unrelated.
• YAML schema consistency verified, including both generated schema copies and the updated contract blob hash.
• Apache RAT license check passed.

API

Yes. The Java and Python skill factories now reject plain HTTP by default and add aligned methods for SHA-256 pinning and explicit HTTP opt-in:

• Java: fromUrlWithSha256, fromUrlUnsafe, and fromUrlUnsafeWithSha256.
• Python: from_url_with_sha256, from_url_unsafe, and from_url_unsafe_with_sha256.

YAML gains a structured url_sources list whose entries accept url, optional sha256, and optional allow_insecure_http. The existing urls list remains available for ordinary HTTPS sources.

Documentation

• doc-needed
• doc-not-needed
• doc-included

Was this patch authored or co-authored using generative AI tooling?

• Yes
• No

Generated-by: OpenAI Codex (GPT-5)

[weiqingy]

Thanks for taking this on. A few questions inline.

[weiqingy]

urlopen here uses the default opener, and its redirect handler allows http, https and ftp targets. So when an HTTPS source redirects to http://…, Python performs the plaintext GET and only rejects it afterwards at :191-193. An ftp:// location gets dialled too, since FTPHandler is installed by default.

No archive bytes are written, so the content is never trusted. But the request itself, URL and headers included, does travel over the downgraded connection. Java never gets that far — HttpURLConnection refuses the cross-protocol redirect, so the second request is never made.

The test covering this (test_materialize.py:164) swaps urlopen for a stub that reports a different geturl(), so it checks the string comparison rather than urllib's redirect handling. This case would slip past it.

What do you think about declining the redirect in a custom opener, so the request is never issued? A real HTTPServer returning a 302 to an http:// location, the way SkillMaterializerTest.java:153 does on the Java side, would cover it end to end.

[weiqingy]

This catches a redirect that changes protocol, but a same-protocol redirect is followed silently and the final URL is never checked. Java only looks at the protocol of the configured URL (:267-272), never at conn.getURL(); Python compares just the scheme of resp.geturl() (_materialize.py:189-196), not the host.

So for an unpinned HTTPS source, an open redirect at the configured host can pull the archive from somewhere else, and nothing records it — SkillOrigin (SkillSourceRegistry.java:56) still reports the URL that was configured. Since the archive becomes agent instructions and possibly scripts, where the bytes actually came from seems worth surfacing.

Pinning a sha256 does catch this. Is that the intended answer, or would logging the final URL when it differs be worth adding?

One related note: the protection against a scheme change here is real but implicit, coming from the JDK declining cross-protocol redirects rather than from this code. A later move to java.net.http.HttpClient would reverse it, since its default policy follows those. Might be worth a comment naming that.

[weiqingy]

The name says BeforeExtraction, but the body only checks the exception and its message. If the digest check moved to after extractZipSafely, the archive would extract, the same IllegalArgumentException with the same "SHA-256 mismatch" text would still be thrown, and this test would stay green. Python's test_sha256_mismatch_rejected (test_url_repository.py:103) has the same gap, just without the name promising more.

Checking before extraction is the property #1003 actually asks for, so a test that turns red when it changes seems worth having. One option: serve an archive that extraction itself would reject — the zip-slip fixture at SkillMaterializerTest.java:72 — and assert the failure is the digest error rather than "Unsafe zip entry". Flip the order and you would get the zip-slip error instead.

Any reason that wouldn't work here?

[weiqingy]

This builds the source without running the URL check, and loader.py:208-210 does the same. So urls: [http://x/s.zip] parses cleanly and only fails later on the TaskManager, while Skills.fromUrl("http://…") fails right away. A malformed sha256 under url_sources behaves the same way.

Both languages do this identically, so it isn't a parity issue. It's that the same mistake surfaces at very different moments depending on how you configure it, and the YAML user finds out last. Nothing on either side tests that a YAML http:// entry is rejected, either.

Was that deliberate, keeping the loaders out of transport policy, or did it just work out that way?

[weiqingy]

urls: [http://…] worked before this PR and now fails, and neither doc says what to switch to. The answer also differs by surface: in code there's from_url_unsafe / fromUrlUnsafe (skills.md:145), but urls has no opt-in at all — the entry has to move into url_sources with allow_insecure_http: true. That's a different shape rather than a flag, so it's the harder one to guess from the error alone.

Something like this after the row, if it helps: "urls entries must be HTTPS. To keep an existing plain-HTTP source working, move it to url_sources with allow_insecure_http: true."

Does that feel like doc material, or more of a release-note thing?

[weiqingy]

nit: this round trip stops at sha256, and test_skills.py::test_serialize_roundtrip does the same, so allow_insecure_http isn't covered on either side.

I compared what the two languages emit and they match exactly — both write the flag as the string "true" and omit it otherwise, and both read it back the same way — so there's no bug here today. The only gap is that nothing would catch it if they drifted. Raising it because #1003 mentions alignment "including plan serialization".

Would adding the flag to this test and its Python counterpart be enough?