Repository navigation
fix(policy-engine): close SSRF literal bypass and fail closed on removed manifest fields after the ACS retarget - #3940
Merged
MohammadHaroonAbuomar merged 8 commits intoSep 13, 2026
Conversation
MohammadHaroonAbuomar
requested review from
liamcrumm and
Prayag (prayagupa)
as code owners
September 13, 2026 21:28
github-actions
Bot
added
size/XL
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
📦 Dependency diff (SBOM)Comparing main → mhabuomar/policy-engine-retarget-followups. ✅ No dependency changes detected. |
`reject_blocked_fetch_host` hand-split the authority and called `str::parse::<IpAddr>`, which accepts only dotted-quad literals. The upstream loader canonicalizes the fetch target with the `url` crate, so `127.1`, `2130706433`, `0x7f000001`, `0177.0.0.1`, `0251.0376.0251.0376` and `127.0.0<TAB>.1` walked past the guard and the fetcher connected to loopback or the metadata address. A loopback listener probe confirmed the TCP connect. Parse with the same `url` crate and evaluate `Url::host()`, so the guard sees the address the fetcher will connect to. Widen the blocked set to private (RFC 1918), shared address space (100.64.0.0/10), IPv6 unique-local and site-local, 0.0.0.0/8, and the names `localhost`, `*.localhost` and `*.local`. Check IPv4-mapped, IPv4-compatible and NAT64 literals on the embedded IPv4 address. A malformed URL now fails closed at the guard. `agent-control-spec` 0.4.0-alpha.3 follows redirects inside its HTTP client and exposes no hook, so hops are still not re-checked. Document that and the `max_manifest_url_redirects: 0` mitigation in the retarget note, the spec and BREAKING_CHANGES.md, and give `manifest_from_url` its own doc comment again. Tests: `manifest_from_url_blocks_ssrf_targets` carries every probe form plus the new ranges and names; `manifest_from_url_never_connects_to_a_ blocked_literal` binds a loopback listener and asserts nothing connects. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit f23315b)
…dropped
`bundle_url`, `system_prompt_file` and `system_prompt_url` were normative
in spec/SPECIFICATION.md and both manifest.schema.json copies, but
agent-control-spec 0.4.0-alpha.3 has no implementation of them and its
policy and annotator configuration maps are open. A manifest declaring
one of them parsed and validated cleanly while the feature was silently
absent: an `llm` annotator ran with the default prompt, and a
`bundle_url` rego policy denied every request with
`runtime_error:policy_invocation_failed` and no diagnostic. Every
fail-closed check the old engine had for them had become a silent
accept.
Add `reject_removed_manifest_fields` to the core shim. It walks each
policy definition, annotator declaration, policy binding and annotation
binding and returns `runtime_error:manifest_invalid` naming the location
and the field, pointing at the migration note. Run it from
`validate_manifest_yaml`, `validate_manifest_overlay_yaml`, every
`AgentControl` constructor, `manifest_from_url`, every C ABI
`acs_builder_from_*` loader, and the Python and Node constructors.
Keep the three keys in both schemas as `not: {}` properties (rego
policy, policy binding, annotator, annotation binding); the enclosing
objects allow additional properties, so dropping the keys would accept
them silently. Update SPECIFICATION.md sections 2.3, 10 and 12.1, the
Foundry example doc, and the stale dispatch-time fetch comments in
host/mod.rs and ffi.rs. Record the change in BREAKING_CHANGES.md and add
a "Removed manifest fields" section and a gap row to acs-retarget.md.
Tests: the six manifests from the review probe are rejection tests in
core (every entry point, plus binding-level and custom-policy forms),
a canary asserts upstream still accepts them so the check can be
retired when it stops, a host constructor test in sdk/rust, a Python
test across validate/from_native/from_manifest_chain, a schema test in
artifact_validation, and two cases in the shared artifact validation
parity corpus that the Rust, Python, Node and .NET runners consume.
Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
(cherry picked from commit 03c11e4)
The retarget note stated an organization or team co-owner on the agent-control-spec crate as a merge precondition. The maintainer waived that in August 2026: the sole owner maintains this integration, publication is bound to a public commit through trusted publishing, and adding a team owner is a registry-side change tracked in upstream #24. Say so instead of presenting it as a blocker, and refresh the verification date and evidence. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit bc6dfd8)
`cargo test --locked -p agent_control_specification` failed to compile under the crate's default features: tests/artifact_validation.rs imports `agent_control_specification_core::validate_acs_artifacts`, which core gates behind `opa`, and the SDK no longer enables that by default. The workspace build only passed because the Python and Node members unify `core/opa` in. Declare the test target with `required-features = ["opa"]` so a per-crate run skips it. The bindings enabled `opa` on core directly, not on the SDK, so with that entry alone the workspace run skipped the test too; enable the SDK's `opa` feature (which forwards to core) from sdk/python and sdk/node so the workspace run keeps it. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 3ca7f91)
`NativeAdapterResult.public_message` still branched on `verdict == "escalate"`, a decision the engine no longer emits. An approval-gated action now arrives as a `deny` carrying an `approval` block, so it fell through to "Request blocked by policy." and the audit record showed a plain deny. Add `approval_required`, read from the verdict's `approval` block, use it for the public message, and include it in the `agt.policy_evaluation.v1` audit record. Tests cover a liftable deny that the host left unresolved and a plain deny. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 4b904c3)
- scripts/check_dependency_confusion.py and sdk/rust/Cargo.toml: cite the pinned agent-control-spec 0.4.0-alpha.3 and agent-hooks-sdk 0.1.0-alpha.5 rather than alpha.1 and alpha.4. - policy-engine/README.md: LICENSE.acs now covers the spec, schema and conformance files; the engine is a registry dependency, not vendored. - Add the Microsoft license header to generator/tests/test_spec_artefacts.py and tests/artifact_enforcement_probe.py. - BREAKING_CHANGES.md: list the host reason-code moves (runtime_error:approval_* to host_error:approval_*, action_mismatch to identity_mismatch, the new approval_unresolved, adapter and streaming codes, the effects codes that went away) and the core module-path moves (manifest_yaml, telemetry_sinks, identity, removed modules, crate-type). Fix the five docs that still cited the old approval codes. - docs/rust-capability-manifest.md: main now depends on the published engine; drop "blocked" and the embedded 0.3.1-beta wording. - scripts/ci/smoke_acs_python_wheel.py: feed the 0.4.0-alpha.1 grammar and validate it, so the release smoke checks the shipped grammar rather than a version the parser does not inspect. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 7417a1f)
Replace the personal name in the ownership note with the role, use a cspell-safe embedded-space host in the SSRF test vector, and add nonblocking to the repo dictionary. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit b3399f0)
MohammadHaroonAbuomar
force-pushed
the
mhabuomar/policy-engine-retarget-followups
branch
from
September 13, 2026 22:27
0aec7b0 to
4193aa5
Compare
github-actions
Bot
removed
dependencies
The Dependency Audit Trail gate requires an audit doc in any PR that changes a lockfile. The follow-up adds url 2.5.8 (already resolved transitively) as a direct dependency of the ACS Rust SDK so the SSRF guard parses hosts with the same crate the fetcher uses. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
MohammadHaroonAbuomar
deleted the
mhabuomar/policy-engine-retarget-followups
branch
September 13, 2026 22:47
This was referenced Sep 13, 2026
Closed
Karim Mehalebi (karimad)
pushed a commit
to karimad/agent-governance-toolkit
that referenced
this pull request
Sep 14, 2026
…ved manifest fields after the ACS retarget (microsoft#3940) * fix(policy-engine): evaluate the SSRF guard on the canonical URL host `reject_blocked_fetch_host` hand-split the authority and called `str::parse::<IpAddr>`, which accepts only dotted-quad literals. The upstream loader canonicalizes the fetch target with the `url` crate, so `127.1`, `2130706433`, `0x7f000001`, `0177.0.0.1`, `0251.0376.0251.0376` and `127.0.0<TAB>.1` walked past the guard and the fetcher connected to loopback or the metadata address. A loopback listener probe confirmed the TCP connect. Parse with the same `url` crate and evaluate `Url::host()`, so the guard sees the address the fetcher will connect to. Widen the blocked set to private (RFC 1918), shared address space (100.64.0.0/10), IPv6 unique-local and site-local, 0.0.0.0/8, and the names `localhost`, `*.localhost` and `*.local`. Check IPv4-mapped, IPv4-compatible and NAT64 literals on the embedded IPv4 address. A malformed URL now fails closed at the guard. `agent-control-spec` 0.4.0-alpha.3 follows redirects inside its HTTP client and exposes no hook, so hops are still not re-checked. Document that and the `max_manifest_url_redirects: 0` mitigation in the retarget note, the spec and BREAKING_CHANGES.md, and give `manifest_from_url` its own doc comment again. Tests: `manifest_from_url_blocks_ssrf_targets` carries every probe form plus the new ranges and names; `manifest_from_url_never_connects_to_a_ blocked_literal` binds a loopback listener and asserts nothing connects. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit f23315b) * fix(policy-engine): fail closed on manifest fields the pinned engine dropped `bundle_url`, `system_prompt_file` and `system_prompt_url` were normative in spec/SPECIFICATION.md and both manifest.schema.json copies, but agent-control-spec 0.4.0-alpha.3 has no implementation of them and its policy and annotator configuration maps are open. A manifest declaring one of them parsed and validated cleanly while the feature was silently absent: an `llm` annotator ran with the default prompt, and a `bundle_url` rego policy denied every request with `runtime_error:policy_invocation_failed` and no diagnostic. Every fail-closed check the old engine had for them had become a silent accept. Add `reject_removed_manifest_fields` to the core shim. It walks each policy definition, annotator declaration, policy binding and annotation binding and returns `runtime_error:manifest_invalid` naming the location and the field, pointing at the migration note. Run it from `validate_manifest_yaml`, `validate_manifest_overlay_yaml`, every `AgentControl` constructor, `manifest_from_url`, every C ABI `acs_builder_from_*` loader, and the Python and Node constructors. Keep the three keys in both schemas as `not: {}` properties (rego policy, policy binding, annotator, annotation binding); the enclosing objects allow additional properties, so dropping the keys would accept them silently. Update SPECIFICATION.md sections 2.3, 10 and 12.1, the Foundry example doc, and the stale dispatch-time fetch comments in host/mod.rs and ffi.rs. Record the change in BREAKING_CHANGES.md and add a "Removed manifest fields" section and a gap row to acs-retarget.md. Tests: the six manifests from the review probe are rejection tests in core (every entry point, plus binding-level and custom-policy forms), a canary asserts upstream still accepts them so the check can be retired when it stops, a host constructor test in sdk/rust, a Python test across validate/from_native/from_manifest_chain, a schema test in artifact_validation, and two cases in the shared artifact validation parity corpus that the Rust, Python, Node and .NET runners consume. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 03c11e4) * docs(policy-engine): record the crates.io ownership decision The retarget note stated an organization or team co-owner on the agent-control-spec crate as a merge precondition. The maintainer waived that in August 2026: the sole owner maintains this integration, publication is bound to a public commit through trusted publishing, and adding a team owner is a registry-side change tracked in upstream microsoft#24. Say so instead of presenting it as a blocker, and refresh the verification date and evidence. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit bc6dfd8) * test(policy-engine): gate the OPA artifact test on the opa feature `cargo test --locked -p agent_control_specification` failed to compile under the crate's default features: tests/artifact_validation.rs imports `agent_control_specification_core::validate_acs_artifacts`, which core gates behind `opa`, and the SDK no longer enables that by default. The workspace build only passed because the Python and Node members unify `core/opa` in. Declare the test target with `required-features = ["opa"]` so a per-crate run skips it. The bindings enabled `opa` on core directly, not on the SDK, so with that entry alone the workspace run skipped the test too; enable the SDK's `opa` feature (which forwards to core) from sdk/python and sdk/node so the workspace run keeps it. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 3ca7f91) * fix(agent-os): report approval-gated denials through the approval block `NativeAdapterResult.public_message` still branched on `verdict == "escalate"`, a decision the engine no longer emits. An approval-gated action now arrives as a `deny` carrying an `approval` block, so it fell through to "Request blocked by policy." and the audit record showed a plain deny. Add `approval_required`, read from the verdict's `approval` block, use it for the public message, and include it in the `agt.policy_evaluation.v1` audit record. Tests cover a liftable deny that the host left unresolved and a plain deny. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 4b904c3) * docs(policy-engine): clear stale retarget references - scripts/check_dependency_confusion.py and sdk/rust/Cargo.toml: cite the pinned agent-control-spec 0.4.0-alpha.3 and agent-hooks-sdk 0.1.0-alpha.5 rather than alpha.1 and alpha.4. - policy-engine/README.md: LICENSE.acs now covers the spec, schema and conformance files; the engine is a registry dependency, not vendored. - Add the Microsoft license header to generator/tests/test_spec_artefacts.py and tests/artifact_enforcement_probe.py. - BREAKING_CHANGES.md: list the host reason-code moves (runtime_error:approval_* to host_error:approval_*, action_mismatch to identity_mismatch, the new approval_unresolved, adapter and streaming codes, the effects codes that went away) and the core module-path moves (manifest_yaml, telemetry_sinks, identity, removed modules, crate-type). Fix the five docs that still cited the old approval codes. - docs/rust-capability-manifest.md: main now depends on the published engine; drop "blocked" and the embedded 0.3.1-beta wording. - scripts/ci/smoke_acs_python_wheel.py: feed the 0.4.0-alpha.1 grammar and validate it, so the release smoke checks the shipped grammar rather than a version the parser does not inspect. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 7417a1f) * chore: satisfy the spell-check gate on the retarget follow-up Replace the personal name in the ownership note with the role, use a cspell-safe embedded-space host in the SSRF test vector, and add nonblocking to the repo dictionary. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit b3399f0) * docs(deps): audit record for the url crate as a direct SDK dependency The Dependency Audit Trail gate requires an audit doc in any PR that changes a lockfile. The follow-up adds url 2.5.8 (already resolved transitively) as a direct dependency of the ACS Rust SDK so the SSRF guard parses hosts with the same crate the fetcher uses. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> --------- Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com>
Imran Siddique (imran-siddique)
added a commit
that referenced
this pull request
Sep 14, 2026
The openshell skill's test fixture manifest declared `agent_control_specification_version: 0.3.0-alpha-agt`. After #3939 retargeted the policy engine onto the published agent-control-spec crate, the runtime refuses it: RuntimeError: runtime_error:manifest_invalid: unsupported agent_control_specification_version '0.3.0-alpha-agt'; supported versions are 0.4.0-alpha.1 This surfaced when main was merged into this branch, which had been 85 commits behind and so had not seen the retarget. Only the version string changes. `extends` and `tools`, which this fixture also uses, remain valid blocks in 0.4.0-alpha.1 per policy-engine/README.md's schema table, so #3940's fail-closed on removed manifest fields does not reach them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QRxFm1Z1kE9iraPspwr7j Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
Yuvraj Singh (yuvrajsingh2428)
pushed a commit
to yuvrajsingh2428/agent-governance-toolkit
that referenced
this pull request
Oct 1, 2026
…ved manifest fields after the ACS retarget (microsoft#3940) * fix(policy-engine): evaluate the SSRF guard on the canonical URL host `reject_blocked_fetch_host` hand-split the authority and called `str::parse::<IpAddr>`, which accepts only dotted-quad literals. The upstream loader canonicalizes the fetch target with the `url` crate, so `127.1`, `2130706433`, `0x7f000001`, `0177.0.0.1`, `0251.0376.0251.0376` and `127.0.0<TAB>.1` walked past the guard and the fetcher connected to loopback or the metadata address. A loopback listener probe confirmed the TCP connect. Parse with the same `url` crate and evaluate `Url::host()`, so the guard sees the address the fetcher will connect to. Widen the blocked set to private (RFC 1918), shared address space (100.64.0.0/10), IPv6 unique-local and site-local, 0.0.0.0/8, and the names `localhost`, `*.localhost` and `*.local`. Check IPv4-mapped, IPv4-compatible and NAT64 literals on the embedded IPv4 address. A malformed URL now fails closed at the guard. `agent-control-spec` 0.4.0-alpha.3 follows redirects inside its HTTP client and exposes no hook, so hops are still not re-checked. Document that and the `max_manifest_url_redirects: 0` mitigation in the retarget note, the spec and BREAKING_CHANGES.md, and give `manifest_from_url` its own doc comment again. Tests: `manifest_from_url_blocks_ssrf_targets` carries every probe form plus the new ranges and names; `manifest_from_url_never_connects_to_a_ blocked_literal` binds a loopback listener and asserts nothing connects. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit f23315b) * fix(policy-engine): fail closed on manifest fields the pinned engine dropped `bundle_url`, `system_prompt_file` and `system_prompt_url` were normative in spec/SPECIFICATION.md and both manifest.schema.json copies, but agent-control-spec 0.4.0-alpha.3 has no implementation of them and its policy and annotator configuration maps are open. A manifest declaring one of them parsed and validated cleanly while the feature was silently absent: an `llm` annotator ran with the default prompt, and a `bundle_url` rego policy denied every request with `runtime_error:policy_invocation_failed` and no diagnostic. Every fail-closed check the old engine had for them had become a silent accept. Add `reject_removed_manifest_fields` to the core shim. It walks each policy definition, annotator declaration, policy binding and annotation binding and returns `runtime_error:manifest_invalid` naming the location and the field, pointing at the migration note. Run it from `validate_manifest_yaml`, `validate_manifest_overlay_yaml`, every `AgentControl` constructor, `manifest_from_url`, every C ABI `acs_builder_from_*` loader, and the Python and Node constructors. Keep the three keys in both schemas as `not: {}` properties (rego policy, policy binding, annotator, annotation binding); the enclosing objects allow additional properties, so dropping the keys would accept them silently. Update SPECIFICATION.md sections 2.3, 10 and 12.1, the Foundry example doc, and the stale dispatch-time fetch comments in host/mod.rs and ffi.rs. Record the change in BREAKING_CHANGES.md and add a "Removed manifest fields" section and a gap row to acs-retarget.md. Tests: the six manifests from the review probe are rejection tests in core (every entry point, plus binding-level and custom-policy forms), a canary asserts upstream still accepts them so the check can be retired when it stops, a host constructor test in sdk/rust, a Python test across validate/from_native/from_manifest_chain, a schema test in artifact_validation, and two cases in the shared artifact validation parity corpus that the Rust, Python, Node and .NET runners consume. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 03c11e4) * docs(policy-engine): record the crates.io ownership decision The retarget note stated an organization or team co-owner on the agent-control-spec crate as a merge precondition. The maintainer waived that in August 2026: the sole owner maintains this integration, publication is bound to a public commit through trusted publishing, and adding a team owner is a registry-side change tracked in upstream microsoft#24. Say so instead of presenting it as a blocker, and refresh the verification date and evidence. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit bc6dfd8) * test(policy-engine): gate the OPA artifact test on the opa feature `cargo test --locked -p agent_control_specification` failed to compile under the crate's default features: tests/artifact_validation.rs imports `agent_control_specification_core::validate_acs_artifacts`, which core gates behind `opa`, and the SDK no longer enables that by default. The workspace build only passed because the Python and Node members unify `core/opa` in. Declare the test target with `required-features = ["opa"]` so a per-crate run skips it. The bindings enabled `opa` on core directly, not on the SDK, so with that entry alone the workspace run skipped the test too; enable the SDK's `opa` feature (which forwards to core) from sdk/python and sdk/node so the workspace run keeps it. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 3ca7f91) * fix(agent-os): report approval-gated denials through the approval block `NativeAdapterResult.public_message` still branched on `verdict == "escalate"`, a decision the engine no longer emits. An approval-gated action now arrives as a `deny` carrying an `approval` block, so it fell through to "Request blocked by policy." and the audit record showed a plain deny. Add `approval_required`, read from the verdict's `approval` block, use it for the public message, and include it in the `agt.policy_evaluation.v1` audit record. Tests cover a liftable deny that the host left unresolved and a plain deny. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 4b904c3) * docs(policy-engine): clear stale retarget references - scripts/check_dependency_confusion.py and sdk/rust/Cargo.toml: cite the pinned agent-control-spec 0.4.0-alpha.3 and agent-hooks-sdk 0.1.0-alpha.5 rather than alpha.1 and alpha.4. - policy-engine/README.md: LICENSE.acs now covers the spec, schema and conformance files; the engine is a registry dependency, not vendored. - Add the Microsoft license header to generator/tests/test_spec_artefacts.py and tests/artifact_enforcement_probe.py. - BREAKING_CHANGES.md: list the host reason-code moves (runtime_error:approval_* to host_error:approval_*, action_mismatch to identity_mismatch, the new approval_unresolved, adapter and streaming codes, the effects codes that went away) and the core module-path moves (manifest_yaml, telemetry_sinks, identity, removed modules, crate-type). Fix the five docs that still cited the old approval codes. - docs/rust-capability-manifest.md: main now depends on the published engine; drop "blocked" and the embedded 0.3.1-beta wording. - scripts/ci/smoke_acs_python_wheel.py: feed the 0.4.0-alpha.1 grammar and validate it, so the release smoke checks the shipped grammar rather than a version the parser does not inspect. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit 7417a1f) * chore: satisfy the spell-check gate on the retarget follow-up Replace the personal name in the ownership note with the role, use a cspell-safe embedded-space host in the SSRF test vector, and add nonblocking to the repo dictionary. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> (cherry picked from commit b3399f0) * docs(deps): audit record for the url crate as a direct SDK dependency The Dependency Audit Trail gate requires an audit doc in any PR that changes a lockfile. The follow-up adds url 2.5.8 (already resolved transitively) as a direct dependency of the ACS Rust SDK so the SSRF guard parses hosts with the same crate the fetcher uses. Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> --------- Signed-off-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> Signed-off-by: yuvrajsingh2428 <offcyuvi2428@gmail.com>
4 of 15 tasks
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
1 participant
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.
Related Issue
Follow-up to #3939; depends on it landing first. The branch is cut from
mhabuomar/policy-engine-retarget-acs, so the diff againstmainshows #3939 until that merges. Review the six commits on top of it.Problem & Solution
The retarget review (#3939) found two blockers and a set of follow-ups. This PR fixes them.
1. SSRF literal bypass in
manifest_from_url(security).reject_blocked_fetch_hosthand-split the URL authority and calledstr::parse::<IpAddr>, which only accepts dotted-quad. The upstream loader canonicalizes the fetch target with theurlcrate, so every non-canonical literal walked past the guard and connected.Probe evidence (
cargo test -p agent_control_specification --test zz_ssrf_probeon the #3939 tree, probe file not committed):https://127.1/m.yaml->failed to fetch extends URL 'https://127.0.0.1/m.yaml': io: Connection refused;https://2852039166/m.yaml->'https://169.254.169.254/m.yaml': timeout;https://0251.0376.0251.0376/m.yaml->169.254.169.254; aTcpListeneron127.0.0.1:0fetched throughhttps://127.1:<port>/loggedaccepted connection from 127.0.0.1. Canonical127.0.0.1and[::ffff:7f00:1]were guarded. The error text (Connection refusedvsunexpected end of filevstimeout) is also a host/port oracle.Fix: parse with the same
urlcrate (new exact-pinned direct dependency, already in every lockfile) and evaluateUrl::host(). Blocked set widened to private (RFC 1918), shared address space100.64.0.0/10, IPv6 unique-local and site-local,0.0.0.0/8, and the nameslocalhost,*.localhost,*.local; IPv4-mapped, IPv4-compatible and NAT64 literals are checked on the embedded IPv4 address; a malformed URL fails closed.manifest_from_url_blocks_ssrf_targetsnow carries every probe form (127.1,127.0.1,2130706433,0x7f000001,0x7f.0.0.1,0177.0.0.1,2852039166,0xA9FEA9FE,0251.0376.0251.0376,169.254.43518, trailing dot, embedded tab and newline) plus the new ranges and names, andmanifest_from_url_never_connects_to_a_blocked_literalbinds a loopback listener and asserts nothing connects.Redirects:
agent-control-spec0.4.0-alpha.3 follows them inside its HTTP client (ExtendsFetcherandHttpExtendsFetcherare private,ManifestLoader::with_limits_and_fetcheris#[cfg(test)]), so hops cannot be intercepted. Rather than silently zeroing a limit that Python, Node and the C ABI expose as an explicit parameter, this is documented as a limitation inacs-retarget.md, the spec and BREAKING_CHANGES.md, withmax_manifest_url_redirects: 0as the mitigation. The pre-retarget engine re-checked every hop; that regression stays in upstream #20.2. Fail-open by silent drop:
bundle_url,system_prompt_file,system_prompt_url.Still normative in
spec/SPECIFICATION.mdand bothmanifest.schema.jsoncopies, never implemented upstream (grepof the alpha.3 crate source andgit log -S bundle_urlon the upstream engine are both empty), and swallowed by upstream's openadapter_configandfieldsmaps.Probe evidence:
Manifest::from_yaml_straccepted all six probe manifests (llmsystem_prompt_fileto a missing file; inlinesystem_promptplus unpinned httpsystem_prompt_url; pinned httpssystem_prompt_url; regobundle_urlpinned https; regobundle_urlunpinned http;bundleplusbundle_url).AgentControl::from_manifestplusevaluate_intervention_point(Input)on thebundle_urlmanifest returnedDeny reason=runtime_error:policy_invocation_failedwith no diagnostic; anllmannotator with either prompt field ran withDEFAULT_SYSTEM_PROMPT(upstreamsrc/dispatchers/llm.rsreads onlysystem_prompt/prompt).Fix:
agent_control_specification_core::reject_removed_manifest_fieldswalks each policy definition, annotator declaration, policy binding and annotation binding and returnsruntime_error:manifest_invalidnaming the location and field, pointing at the migration note. It runs invalidate_manifest_yaml,validate_manifest_overlay_yaml, everyAgentControlconstructor,manifest_from_url, every C ABIacs_builder_from_*loader, and the Python and Node constructors. Both schemas keep the three keys asnot: {}properties (rego policy, policy binding, annotator, annotation binding) because the enclosing objects allow additional properties. SPECIFICATION.md sections 2.3, 10 and 12.1 record the removal;acs-retarget.mdgains a "Removed manifest fields" section and a gap-table row; BREAKING_CHANGES.md gains an entry; the Foundry example doc stops recommendingbundle_url. Tests: the six probe manifests as rejection tests through every entry point, binding-level and custom-policy forms, a canary that fails when upstream starts rejecting them (so the AGT check can be retired), a host constructor test, a Python test acrossvalidate_manifest/validate_manifest_overlay/from_native/from_manifest_chain, a schema test, and two cases in the shared artifact-validation parity corpus consumed by the Rust, Python, Node and .NET runners.3. Registry ownership wording.
acs-retarget.mdstated an org/team co-owner on crates.io as a merge blocker. The maintainer waived that in August 2026; the doc now records the decision (sole owner accepted, trusted publishing binds the artifact to a public commit, team owner tracked in upstream responsibleai/agent-control-spec#24) with refreshed evidence.4. Cheap follow-ups.
cargo test --locked -p agent_control_specification(default features) failed to compile:tests/artifact_validation.rsneeds core'sopa. Declaredrequired-features = ["opa"]; the bindings now enable the SDK'sopafeature (which forwards to core) so the workspace run still includes the test.agent_os/integrations/_native_adapter_runtime.pybranched onverdict == "escalate", which the engine never emits. Addedapproval_requiredfrom the verdict'sapprovalblock, used it for the public message, added it to theagt.policy_evaluation.v1audit record, with tests.scripts/check_dependency_confusion.pyandsdk/rust/Cargo.toml(alpha.1/alpha.4 -> alpha.3/alpha.5).policy-engine/README.mdLICENSE.acs wording (registry dependency now; notice covers spec, schema and conformance files).generator/tests/test_spec_artefacts.pyandtests/artifact_enforcement_probe.py.runtime_error:approval_*->host_error:approval_*,action_mismatch->identity_mismatch, newapproval_unresolved, adapter/streaming codes, retired effect codes) and core module-path moves (manifest_yaml,telemetry_sinks,identity, removed modules,crate-type). Fixed the five docs still citing the old approval codes.docs/rust-capability-manifest.md: main now depends on the published engine; dropped "blocked" and the embedded 0.3.1-beta wording.scripts/ci/smoke_acs_python_wheel.py: feeds the 0.4.0-alpha.1 grammar and validates it.Left for issues
Not implemented here; each needs its own issue:
AgentControl::new(runtime)leavesparts: None, sowith_telemetryis a silent no-op and transformed snapshots revalidate againstLimits::default()instead of the caller's limits. Also bypassesreject_removed_manifest_fieldswhen the caller built theRuntimeby hand.acs_builder_set_url_fetch_limitsis write-only (builder.limitsnever read);acs_builder_from_urlfetches with the default budget.core/tests/ffi_roundtrip.rsandffi_default_dispatchers.rstosdk/rust/tests/.bundled-dispatchersreject any manifest declaring annotators unlessannotator_dispatcheris passed; fail-closed and deliberate, but absent from BREAKING_CHANGES.md.regoandstreaming.=0.3.1-beta.0. Bump before any Rust release (same story for the npm family).bundle,dataanddata_pathspaths; upstream skips path resolution forManifestLocation::Urlbut does not reject the fields (noted inacs-retarget.md, needs upstream build(deps-dev): Bump @types/uuid from 9.0.8 to 11.0.0 in /packages/agent-os/extensions/mcp-server #20).Impact on Your Work
Closes the two findings that block #3939 from being called safe: a guard that did not guard, and a spec/schema that promised features the engine silently dropped.
Timeline
Merge after #3939.
Alternatives Considered
max_manifest_url_redirectsto 0 insidemanifest_from_url. Rejected: it would silently override an explicit parameter in three SDKs; documented instead, with the mitigation.not: {}.#![cfg(feature = "opa")]on the test file instead ofrequired-features. Equivalent; the binding-side feature change was needed either way so the workspace run keeps the test.Type of Change
Package(s) Affected
Core & runtime:
Governance & security:
Platform & tooling:
CLI plugins:
Shared / other:
Testing
Unit Testing
sdk/rust:manifest_from_url_blocks_ssrf_targets(every probe form, new ranges and names, malformed URLs, public destinations pass),manifest_from_url_never_connects_to_a_blocked_literal(loopback listener),host_constructors_reject_removed_manifest_fields.core:removed_fields_are_rejected_by_every_validation_entry_point(the six probe manifests),removed_fields_are_rejected_on_bindings_too,upstream_parser_still_accepts_the_removed_fields(canary),manifests_without_removed_fields_still_pass,manifest_schema_rejects_removed_fields.tests/parity/artifact-validation-cases.json:removed_bundle_url_field,removed_system_prompt_file_field(Rust, Python, Node, .NET runners).sdk/python:test_removed_manifest_fields_fail_closed_everywhere.agent-os:test_native_result_routes_liftable_deny_to_the_approval_message,test_native_result_plain_deny_is_not_approval_required.Manual Testing
All run on this branch, offline registry,
opaon PATH:cargo fmt --all --checkclean;cargo clippy --locked --workspace --all-targets -D warningsclean;cargo test --locked --workspace123 passed, 0 failed (includesartifact_validation);cargo test --locked -p agent_control_specification(default features) 67 passed, compiles again;cargo check --locked -p agent_control_specification_core --no-default-featuresclean.cargo test --locked516 passed.examples/coding_agent/app:cargo metadata --lockedclean.npm ci && npm test: 126 tests, 125 pass, 1 skipped.dotnet build --configuration Release0 errors; test runner all groups passed.scripts/docs/check_frontmatter.py --root . --strict0 findings;scripts/docs/check_links.py --root .0 new broken;scripts/check_license_headers.pyon changed files clean;scripts/check_dependency_confusion.py --strictexit 0;scripts/check_release_age.py --base origin/mainall 9 OK (url 2.5.8 released 2026-01-05);scripts/ci/generate_workflows.py --check,scripts/sync-version.py --check,scripts/check_v4_ratchet.py,scripts/ci/vendored-patch-audit.sh origin/mainall pass.Checklist
Attribution & Prior Art
Prior art / related projects (if any):
The guard shape follows the pre-retarget engine's
validate_url_componentsonmain(url::Url::host()match), which this restores and extends.AI Assistance
If AI tools materially shaped this change, briefly note what was used:
IP, Patents, and Licensing