fix(policy-engine): reject local bundle and data paths on URL-sourced manifests by wangyusheng1985 · Pull Request #4004 · microsoft/agent-governance-toolkit

玉升 (wangyusheng1985) · 2026-09-15T13:51:16Z

Closes #3947.

The pinned agent-control-spec loader skips relative path resolution for a URL sourced manifest. A remote document could therefore declare bundle, data, data_paths, or Cedar policy_path / entities_path / schema_path and have those names resolved against the process working directory. SPECIFICATION.md 2.3 requires fail closed.

manifest_from_url now runs reject_url_sourced_local_paths after the fetch. File-sourced constructors still accept local paths. Inline query is unchanged because it is not a filesystem path field.

origin/main manifest_from_url does not call this rejection. rustfmt --edition 2021 --check passed on the touched Rust files.

This change was produced with AI assistance.

Validation observed for this change:

rustfmt --edition 2021 --check /workspace/repository/policy-engine/core/src/manifest_yaml.rs /workspace/repository/policy-engine/sdk/rust/src/host/mod.rs /workspace/repository/policy-engine/sdk/rust/src/host/tests.rs /workspace/repository/policy-engine/sdk/rust/src/lib.rs /workspace/repository/policy-engine/core/src/lib.rs
`python3 - <<'PY'
from pathlib import Path

core = Path('/workspace/repository/policy-engine/core/src/manifest_yaml.rs').read_text()
host = Path('/workspace/repository/policy-engine/sdk/rust/src/host/mod.rs').read_text()
host_tests = Path('/workspace/repository/policy-engine/sdk/rust/src/host/tests.rs').read_text()
spec = Path('/workspace/repository/policy-engine/spec/SPECIFICATION.md').read_text()
sdk = Path('/workspace/repository/policy-engine/sdk/rust/src/lib.rs').read_text()

assert 'fn reject_url_sourced_local_paths' in core
assert 'pub const URL_SOURCED_FILESYSTEM_FIELDS: [&str; 6]' in core
for field in ('bundle', 'data', 'data_paths', 'policy_path', 'entities_path', 'schema_path'):
assert f'"{field}"' in core
assert '"query"' not in core[core.index('URL_SOURCED_FILESYSTEM_FIELDS'):core.index('pub fn reject_url_sourced_local_paths')]
assert 'reject_url_sourced_local_paths(&manifest)?' in host
assert 'fn manifest_from_url' in host
assert 'reject_url_sourced_local_paths' in sdk
assert "url_sourced_loader_rejects_local_bundle_and_data_paths" in host_tests
assert "policy 'p' declares 'bundle'" in host_tests
assert "policy 'p' declares 'data_paths'" in host_tests
assert 'url_sourced_manifests_reject_filesystem_path_fields' in core
assert 'entities_path' in core
assert 'schema_path' in core
assert 'MUST NOT carry a filesystem path field' in spec
print('URL-sourced filesystem path rejection is wired in core, host loader, tests, and SPEC 2.3')
PY`

Fixes #3947

Signed-off-by: wangyusheng1985 <wangyusheng1985@users.noreply.github.com>

azure-pipelines · 2026-09-15T13:51:26Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

github-actions · 2026-09-15T13:51:27Z

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

github-actions · 2026-09-15T13:51:28Z

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

github-actions · 2026-09-15T13:52:04Z

🟡 Contributor Check: MEDIUM

Check	Result
Profile	MEDIUM
Credential	LOW
Overall	MEDIUM

Automated check by AGT Contributor Check.

MohammadHaroonAbuomar

Sign the Microsoft CLA (license/cla is QUEUED; microsoft-github-policy-service asked at 13:52Z). The PR is also still a draft; mark ready once the test passes.

MohammadHaroonAbuomar · 2026-09-15T15:31:35Z

+            (
+                "entities_path",
+                cedar_manifest("    entities_path: ./entities.json\n"),
+            ),
+            (
+                "schema_path",
+                cedar_manifest("    schema_path: ./schema.json\n"),
+            ),


803 (test url_sourced_manifests_reject_filesystem_path_fields): the entities_path and schema_path cases build a Cedar policy with neither policy_set nor policy_path, so upstream Manifest::from_yaml_str rejects them (cedar policies must declare exactly one of policy_set or policy_path) and the helper at line 729 panics before the new check runs. cargo test -p agent_control_specification_core --lib manifest_yaml fails: 5 passed, 1 failed. Fix: pass " policy_set: |\n permit(principal, action, resource);\n entities_path: ./entities.json\n" (and the same for schema_path) to cedar_manifest, as the accepted Cedar case at line 826 already does, then re-run the command and replace the Python string-assertion script in the PR body with the actual cargo test output.

Thanks for the review. I’ll address this in the existing pull request and rerun the focused validation.

MohammadHaroonAbuomar · 2026-09-15T15:31:35Z

+still skips relative path resolution for a URL sourced manifest. AGT's
+`manifest_from_url` then rejects filesystem path fields (`bundle`, `data`,
+`data_paths`, Cedar `policy_path`, `entities_path`, `schema_path`) so a remote
+manifest cannot name local files. Closing the same gap inside


the sentence AGT's manifest_from_url then rejects filesystem path fields ... so a remote manifest cannot name local files overstates the fix. Only manifest_from_url (and the Python/Node/C-ABI from_url loaders that call it) run the check. A file-sourced manifest whose extends names an HTTPS URL still receives the remote parent's bundle/data/data_paths/Cedar path fields unresolved (upstream load_location_with_body skips resolve_relative_paths for ManifestLocation::Url, and after merge AGT cannot tell which fields came from the URL). Reword to: a manifest loaded through manifest_from_url cannot name local files; a file-sourced manifest with a URL extends entry still can, which needs the upstream fix. Same scoping for the CHANGELOG.md:28 entry.

Thanks for the review. I’ll address this in the existing pull request and rerun the focused validation.

azure-pipelines · 2026-09-16T12:53:14Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

MohammadHaroonAbuomar · 2026-09-17T09:40:14Z

+            (
+                "entities_path",
+                cedar_manifest("    entities_path: ./entities.json\n"),
+            ),
+            (
+                "schema_path",
+                cedar_manifest("    schema_path: ./schema.json\n"),
+            ),


803 (test url_sourced_manifests_reject_filesystem_path_fields): the entities_path and schema_path cases still build a Cedar policy with neither policy_set nor policy_path; upstream Manifest::from_yaml_str rejects them and the helper at line 729 panics before reject_url_sourced_local_paths runs. cargo test -p agent_control_specification_core --lib manifest_yaml at 2256530: 5 passed, 1 failed. Fix: pass " policy_set: |\n permit(principal, action, resource);\n entities_path: ./entities.json\n" (and the same for schema_path) to cedar_manifest, as the accepted Cedar case at line 826 already does; re-run the command and replace the Python string-assertion script in the PR body with the cargo test output.

Thanks for the review. I’ll address this in the existing pull request and rerun the focused validation.

MohammadHaroonAbuomar · 2026-09-17T09:40:14Z

+still skips relative path resolution for a URL sourced manifest. AGT's
+`manifest_from_url` then rejects filesystem path fields (`bundle`, `data`,
+`data_paths`, Cedar `policy_path`, `entities_path`, `schema_path`) so a remote
+manifest cannot name local files. Closing the same gap inside


273 and CHANGELOG.md:28 still claim a remote manifest cannot name local files / URL-sourced ACS manifests cannot name local policy files. Only manifest_from_url (and the Python/Node/C-ABI from_url loaders that call it) run the check; a file-sourced manifest whose extends names an HTTPS URL still receives the remote parent's bundle/data/data_paths/Cedar path fields unresolved. Reword both to: a manifest loaded through manifest_from_url cannot name local files; a file-sourced manifest with a URL extends entry still can, which needs the upstream fix.

Thanks for the review. I’ll address this in the existing pull request and rerun the focused validation.

fix(policy-engine): reject local paths on URL-sourced manifests

7f2be4e

Signed-off-by: wangyusheng1985 <wangyusheng1985@users.noreply.github.com>

github-actions Bot added documentation Improvements or additions to documentation size/L Large PR (< 500 lines) labels Sep 15, 2026

github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Sep 15, 2026

MohammadHaroonAbuomar requested changes Sep 15, 2026

View reviewed changes

玉升 (wangyusheng1985) marked this pull request as ready for review September 16, 2026 12:53

玉升 (wangyusheng1985) requested review from liamcrumm and Prayag (prayagupa) as code owners September 16, 2026 12:53

Merge branch 'main' into repo-agent/cb145603-3947

2256530

MohammadHaroonAbuomar requested changes Sep 17, 2026

View reviewed changes

Conversation

玉升 (wangyusheng1985) commented Sep 15, 2026

Uh oh!

azure-pipelines Bot commented Sep 15, 2026

Uh oh!

github-actions Bot commented Sep 15, 2026

PR Review Summary

Uh oh!

github-actions Bot commented Sep 15, 2026

Uh oh!

github-actions Bot commented Sep 15, 2026

Uh oh!

MohammadHaroonAbuomar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azure-pipelines Bot commented Sep 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants