diff --git a/policies/rust/now-policy-api/openapi/now-policy-api.yaml b/policies/rust/now-policy-api/openapi/now-policy-api.yaml index 80eff14..ba875ec 100644 --- a/policies/rust/now-policy-api/openapi/now-policy-api.yaml +++ b/policies/rust/now-policy-api/openapi/now-policy-api.yaml @@ -746,11 +746,26 @@ components: format: date-time additionalProperties: false PackageIdentifier: - description: Package identifier string. + description: |- + Package identifier string. + + Validated against an explicit allowlist of characters: ASCII alphanumerics plus `. - _ + @ / : [ ] , # $ % { }`. + + - `.`, `-`, `_`, `+`: winget (`Notepad++.Notepad++`), chocolatey, pip, cargo, dotnet, apt/dnf/pacman (`g++`, `libstdc++6`), PowerShell modules; + + - `@`, `/`: scoped npm/Bun packages (`@scope/package`), homebrew and scoop `tap/formula` paths, versioned formulas (`python@3.11`); + + - `:`: npm aliases (`alias:@scope/package@1.0.0`), vcpkg triplets (`curl:x64-windows`); + + - `[`, `]`, `,`: vcpkg features (`curl[ssl,http2]:x64-windows`), pip extras (`requests[socks]`); + + - `#`, `$`, `%`, `{`, `}`: additional identifier punctuation (accepted by product decision for forward compatibility). Caveat: these characters carry expansion semantics in some shells (`${VAR}`, `%VAR%`, brace expansion), so downstream command builders must pass identifiers as discrete process arguments and never interpolate them into a shell command line. + + Version range/pin operators (`<`, `>`, `=`, `!`, `|`, `^`, `~`) are rejected: the broker matches against a specific, exact version carried in the request's separate `Package.Version` field, so range expressions do not belong in the identifier (npm aliases must use exact versions, e.g. `alias:pkg@7.20.0`). The wildcards `*` and `?` are also rejected: policy-side package identifier matching is wildcard-based, so wildcards in request identifiers would be ambiguous. Everything else — whitespace, control characters, `"`, `\`, backtick, `& ' ( ) ;`, and non-ASCII — is rejected as well. type: string maxLength: 256 minLength: 1 - pattern: ^[^\\\/:*?"<>|\x01-\x1f]+$ + pattern: ^[A-Za-z0-9._+@/:\[\],#$%{}-]+$ PackageRequest: description: Canonical request sent by a package broker client to the elevated broker. type: object diff --git a/policies/rust/now-policy-api/src/lib.rs b/policies/rust/now-policy-api/src/lib.rs index f194863..4b6b8d3 100644 --- a/policies/rust/now-policy-api/src/lib.rs +++ b/policies/rust/now-policy-api/src/lib.rs @@ -398,6 +398,38 @@ impl From<&str> for RuleId { } /// Package identifier string. +/// +/// Validated against an explicit allowlist of characters: ASCII alphanumerics +/// plus `. - _ + @ / : [ ] , # $ % { }`. +/// +/// - `.`, `-`, `_`, `+`: winget (`Notepad++.Notepad++`), chocolatey, pip, cargo, +/// dotnet, apt/dnf/pacman (`g++`, `libstdc++6`), PowerShell modules; +/// +/// - `@`, `/`: scoped npm/Bun packages (`@scope/package`), homebrew and scoop +/// `tap/formula` paths, versioned formulas (`python@3.11`); +/// +/// - `:`: npm aliases (`alias:@scope/package@1.0.0`), vcpkg triplets +/// (`curl:x64-windows`); +/// +/// - `[`, `]`, `,`: vcpkg features (`curl[ssl,http2]:x64-windows`), pip extras +/// (`requests[socks]`); +/// +/// - `#`, `$`, `%`, `{`, `}`: additional identifier punctuation (accepted by +/// product decision for forward compatibility). Caveat: these characters +/// carry expansion semantics in some shells (`${VAR}`, `%VAR%`, brace +/// expansion), so downstream command builders must pass identifiers as +/// discrete process arguments and never interpolate them into a shell +/// command line. +/// +/// Version range/pin operators (`<`, `>`, `=`, `!`, `|`, `^`, `~`) are +/// rejected: the broker matches against a specific, exact version carried in +/// the request's separate `Package.Version` field, so range expressions do +/// not belong in the identifier (npm aliases must use exact versions, e.g. +/// `alias:pkg@7.20.0`). The wildcards `*` and `?` are also rejected: +/// policy-side package identifier matching is wildcard-based, so wildcards in +/// request identifiers would be ambiguous. Everything else — whitespace, +/// control characters, `"`, `\`, backtick, `& ' ( ) ;`, and non-ASCII — is +/// rejected as well. #[derive( Debug, Clone, @@ -414,7 +446,7 @@ impl From<&str> for RuleId { #[deref(forward)] #[display("{_0}")] pub struct PackageIdentifier( - #[schemars(length(min = 1, max = 256), regex(pattern = r#"^[^\\\/:*?"<>|\x01-\x1f]+$"#))] pub String, + #[schemars(length(min = 1, max = 256), regex(pattern = r"^[A-Za-z0-9._+@/:\[\],#$%{}-]+$"))] pub String, ); impl PackageIdentifier { @@ -433,21 +465,29 @@ impl PackageIdentifier { }); } - if s.bytes().any(|b| { - b == b'\\' - || b == b'/' - || b == b':' - || b == b'*' - || b == b'?' - || b == b'"' - || b == b'<' - || b == b'>' - || b == b'|' - || (0x01..=0x1f).contains(&b) + if !s.bytes().all(|b| { + b.is_ascii_alphanumeric() + || matches!( + b, + b'.' | b'-' + | b'_' + | b'+' + | b'@' + | b'/' + | b':' + | b'[' + | b']' + | b',' + | b'#' + | b'$' + | b'%' + | b'{' + | b'}' + ) }) { return Err(ModelValidationError::Invalid { type_name: "PackageIdentifier", - reason: "contains forbidden characters".to_owned(), + reason: "must contain only ASCII alphanumerics or '. - _ + @ / : [ ] , # $ % { }'".to_owned(), }); } @@ -542,3 +582,160 @@ impl<'de> Deserialize<'de> for CommandString { Self::parse(&s).map_err(serde::de::Error::custom) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn package_identifier_accepts_manager_specific_identifiers() { + let valid = [ + // WinGet. + "Microsoft.VisualStudioCode", + "Notepad++.Notepad++", + // Scoped npm/Bun packages. + "@scope/package", + "@babel/core", + // npm aliases (exact versions only). + "babel-core-legacy:@babel/core@7.20.0", + "my-alias:lodash@4.17.21", + "pinned:react@18.2.0", + // vcpkg triplets and features. + "curl:x64-windows", + "curl[ssl]:x64-windows", + "curl[ssl,http2]:x64-windows", + // Homebrew/scoop tap paths and versioned formulas. + "extras/vscode", + "homebrew/core/python@3.11", + // pip extras. + "requests[socks]", + // Chocolatey, dotnet, cargo, apt-style names. + "git.install", + "dotnet-ef", + "serde_json", + "g++", + // Additional identifier punctuation. + "foo#bar", + "foo$bar", + "foo%20bar", + "foo{bar}", + ]; + + for id in valid { + PackageIdentifier::parse(id).unwrap_or_else(|e| panic!("{id:?} should be valid: {e}")); + + let json = serde_json::to_string(id).expect("string should serialize to JSON"); + let deserialized: PackageIdentifier = + serde_json::from_str(&json).unwrap_or_else(|e| panic!("{id:?} should deserialize: {e}")); + assert_eq!(deserialized.0, id); + } + } + + #[test] + fn package_identifier_rejects_forbidden_input() { + let invalid = [ + "", + "foo\\bar", + "foo\"bar", + "foo*", + "foo?", + "foo\r\nbar", + "foo\tbar", + "foo bar", + "foo\u{0}bar", + "foo\u{7f}bar", + "foo&bar", + "foo'bar", + "foo(bar)", + "foo;bar", + "foo`bar", + // Version range/pin operators (exact versions only, carried in Package.Version). + "babel-core-legacy:@babel/core@^7.20.0", + "my-alias:lodash@~4.17.21", + "pinned:react@=18.2.0", + "requests==2.32.0", + "requests!=2.32.0", + "ranged:react@>=18.2.0", + "either:foo@1.0.0||2.0.0", + "spec<2.0", + "gr\u{fc}n", + &"a".repeat(257), + ]; + + for id in invalid { + PackageIdentifier::parse(id).expect_err(&format!("{id:?} should be rejected")); + + let json = serde_json::to_string(id).expect("string should serialize to JSON"); + serde_json::from_str::(&json) + .expect_err(&format!("{id:?} should be rejected during deserialization")); + } + } + + #[test] + fn package_request_deserializes_scoped_and_aliased_identifiers() { + let identifiers = [ + "@scope/package", + "babel-core-legacy:@babel/core@7.20.0", + "curl:x64-windows", + "curl[ssl]:x64-windows", + ]; + + for id in identifiers { + let json = serde_json::json!({ + "RequestKind": "PackageRequest", + "RequestVersion": "1.0", + "RequestId": "req-scoped-install", + "CreatedAt": "2026-05-05T12:00:00Z", + "Operation": "Install", + "Manager": "Npm", + "Source": { "Name": "npm" }, + "Package": { "Id": id }, + "Options": { + "Interactive": false, + "SkipHashCheck": false, + "PreRelease": false + }, + "Client": { + "RequestedElevation": "Elevated", + "EffectiveUser": "CONTOSO\\alice", + "ClientVersion": "3.2.0", + "Transport": "HttpNamedPipe", + "ClientExecutablePath": "C:\\Program Files\\Devolutions\\NOW\\now-client.exe" + } + }); + + let request: PackageRequest = serde_json::from_value(json) + .unwrap_or_else(|e| panic!("PackageRequest with id {id:?} should deserialize: {e}")); + assert_eq!(request.package.id.0, id); + } + } + + #[test] + fn package_request_rejects_invalid_identifier() { + let json = serde_json::json!({ + "RequestKind": "PackageRequest", + "RequestVersion": "1.0", + "RequestId": "req-invalid-id", + "CreatedAt": "2026-05-05T12:00:00Z", + "Operation": "Install", + "Manager": "Winget", + "Source": { "Name": "winget" }, + "Package": { "Id": "evil;package\r\n" }, + "Options": { + "Interactive": false, + "SkipHashCheck": false, + "PreRelease": false + }, + "Client": { + "RequestedElevation": "Elevated", + "EffectiveUser": "CONTOSO\\alice", + "ClientVersion": "3.2.0", + "Transport": "HttpNamedPipe", + "ClientExecutablePath": "C:\\Program Files\\Devolutions\\NOW\\now-client.exe" + } + }); + + serde_json::from_value::(json) + .expect_err("PackageRequest with forbidden identifier characters should be rejected"); + } +}