Skip to content

fix(policy): constrain schema version digits - #108

Closed
Benoît Cortier (CBenoit) wants to merge 1 commit into
masterfrom
cbenoit-policy-version-schema
Closed

Benoît Cortier (CBenoit) wants to merge 1 commit into
masterfrom
cbenoit-policy-version-schema

Conversation

@CBenoit

Copy link
Copy Markdown
Member

Keeps PolicyFormatVersion schemas readable while conservatively rejecting impractically large numeric components. Runtime parsing continues to use u64 bounds and may therefore be stricter than the schema.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 15, 2026 12:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The 19-digit limit rejects runtime-supported 20-digit u64 values and contradicts the documented schema relationship.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Constrains PolicyFormatVersion numeric components across policy schemas and OpenAPI.

Changes:

  • Adds digit limits to Rust-generated schema patterns.
  • Updates generated JSON/OpenAPI contracts.
  • Adds a .NET schema boundary test.
File summaries
File Description
policies/rust/now-policy/src/newtypes.rs Defines the constrained schema regex.
policies/rust/now-policy/schema/devolutions.now-policy.schema.json Updates the committed-policy schema.
policies/rust/now-policy/schema/devolutions.now-policy-draft.schema.json Updates the draft-policy schema.
policies/rust/now-policy-api/openapi/now-policy-api.yaml Synchronizes OpenAPI patterns.
policies/dotnet/Devolutions.Now.Policy.Model.Tests/PolicyTests.cs Tests schema rejection at the numeric boundary.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread policies/dotnet/Devolutions.Now.Policy.Model.Tests/PolicyTests.cs
Comment thread policies/rust/now-policy/src/newtypes.rs
@CBenoit

Copy link
Copy Markdown
Member Author

Closing as superseded by the clarified contract: runtime readers use u64 components, while the intentionally simple schema may accept larger numeric strings. The merged implementation already has that behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants