test(agent): validate broker package ids over the wire

[Vladyslav Nikonov (vnikonov-devolutions)]

Broker tests constructed PackageIdentifier directly, bypassing the now-policy-api wire validation. This hid the fact that identifiers supported by the command builders, such as scoped npm packages, npm aliases, and vcpkg triplets, were rejected during request deserialization by now-policy-api 0.3.0.

Construct identifiers through PackageIdentifier::parse so every identifier used in positive-path tests is proven to be accepted over the wire. Deliberately malicious identifiers keep direct construction, documented as defense-in-depth checks, and a new test asserts that control characters are rejected at the wire boundary.

---

Blocked on release: the relaxed identifier validation lands in Devolutions/now-libraries#91 and will be published as now-policy-api 0.3.1 by release-plz. Until then, tests using / or : in identifiers fail against the published 0.3.0. Cargo.toml stays at 0.3; a cargo update -p now-policy-api will pick up 0.3.1 once released. This section should be removed before merge.

[github-actions]

Let maintainers know that an action is required on their side

• Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)

• Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release

• Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)

• Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

[Vladyslav Nikonov (vnikonov-devolutions)]

Implementation notes:

• Positive-path tests now build identifiers with PackageIdentifier::parse(...).expect(...), so any identifier a builder test relies on is proven to pass request deserialization (parse and Deserialize share the same validation in now-policy-api).
• Negative-path injection tests (npm contoso\r\nnpm run unsafe, pip requirement injection, vcpkg invalid specs) deliberately keep direct From<String> construction with a comment: they exercise the builders' defense-in-depth rejection for identifiers that bypass deserialization. Some of those inputs (e.g. " , \, control chars) are also rejected at the wire, which is exactly why direct construction is required there.
• Added wire_validation_rejects_line_separators in npm.rs asserting PackageIdentifier::parse rejects control characters, pairing the builder-level check with a wire-level one.
• Verified locally against the now-libraries fix branch (fix(now-policy-api): allow '/' and ':' in PackageIdentifier now-libraries#91) via a temporary [patch.crates-io] (not committed): 251 broker tests pass, clippy clean, rustfmt applied. Round-trip wire tests for scoped/alias/triplet identifiers live in the now-policy-api crate itself.

Note

LLM-assisted content (no human feedback).

[Copilot]

Pull request overview

Updates broker tests to validate package identifiers through PackageIdentifier::parse, while retaining direct construction for defense-in-depth cases.

Changes:

• Validates positive-path test identifiers using wire-equivalent parsing.
• Documents intentional validation bypasses in malicious-input tests.
• Adds control-character rejection coverage.
• Currently blocked by now-policy-api 0.3.1 availability and lockfile update.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
devolutions-agent/src/broker/server/mod.rs	Parses the server test identifier.
devolutions-agent/src/broker/evaluator/tests.rs	Parses evaluator test identifiers.
devolutions-agent/src/broker/evaluator/matching.rs	Parses the matching fixture identifier.
devolutions-agent/src/broker/evaluator/constraints.rs	Parses the constraints fixture identifier.
devolutions-agent/src/broker/command_builder/winget.rs	Parses the WinGet identifier.
devolutions-agent/src/broker/command_builder/vcpkg.rs	Validates vcpkg identifiers and documents bypass cases.
devolutions-agent/src/broker/command_builder/scoop.rs	Parses the Scoop identifier.
devolutions-agent/src/broker/command_builder/powershell.rs	Parses PowerShell package identifiers.
devolutions-agent/src/broker/command_builder/pip.rs	Parses valid pip identifiers and documents malicious cases.
devolutions-agent/src/broker/command_builder/npm.rs	Validates npm aliases and control-character rejection.
devolutions-agent/src/broker/command_builder/dotnet.rs	Parses the .NET package identifier.
devolutions-agent/src/broker/command_builder/chocolatey.rs	Parses the Chocolatey identifier.
devolutions-agent/src/broker/command_builder/cargo.rs	Parses Cargo test identifiers.
devolutions-agent/src/broker/command_builder/bun.rs	Validates scoped Bun identifiers.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Vladyslav Nikonov (vnikonov-devolutions)]

Known and intentional — this PR is a draft blocked on the now-policy-api 0.3.1 release (Devolutions/now-libraries#91), as noted in the PR description. The suite was verified locally against the fix branch via a temporary [patch.crates-io] (251 broker tests pass). Once 0.3.1 is published, the lockfile will be updated with cargo update -p now-policy-api and CI re-verified before marking ready for review.

Note

LLM-assisted content (no human feedback).