Skip to content

feat: make policy length limits configurable per engine#624

Merged
anakrish merged 1 commit intomicrosoft:mainfrom
antmhs:custom-policy-limits
Mar 13, 2026
Merged

feat: make policy length limits configurable per engine#624
anakrish merged 1 commit intomicrosoft:mainfrom
antmhs:custom-policy-limits

Conversation

@antmhs
Copy link
Contributor

@antmhs antmhs commented Mar 12, 2026

Trying to update to the latest regorus version pulled in #531 which broke some things on my end. This change makes it so the policy length limits are configurable (the limits from #531 are still the default, though).

Implemented:

  • Add PolicyLengthConfig struct with max_col, max_file_bytes, and max_lines fields, replacing hardcoded constants in the lexer.
  • Add Engine::set_policy_length_config and clear_policy_length_config to allow callers to override the default limits.
  • Add Source::from_contents_with_limits and from_file_with_limits for direct Source construction with custom limits; existing from_contents and from_file signatures are preserved using defaults.
  • Add tests for default rejection, custom limits, and engine plumbing.

@antmhs
Copy link
Contributor Author

antmhs commented Mar 12, 2026

@microsoft-github-policy-service agree company="Amazon"

Copy link
Collaborator

@anakrish anakrish left a comment

Choose a reason for hiding this comment

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

@antmhs Thanks for the contribution.

It would be worth adding a comment in new_from_compiled_policy that since compiled policies are already parsed, they don't need length limit config.

@antmhs antmhs force-pushed the custom-policy-limits branch from 8760832 to f15e958 Compare March 12, 2026 15:58
@antmhs
Copy link
Contributor Author

antmhs commented Mar 12, 2026

I added setters to Parser and Lexer instead since I saw that that's how the Rego V0 field is passed too. Also, the constructors are already used in a few places in the examples directory.

Copy link
Collaborator

@anakrish anakrish left a comment

Choose a reason for hiding this comment

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

@antmhs Looks good. Some more feedback about comments and test.

Ideally, we would want to also expose this new config to bindings. But we can do that in a second PR. Are you primarily a Rust based client? Another user from Amazon was mainly Java based.

@antmhs antmhs force-pushed the custom-policy-limits branch from f15e958 to d495293 Compare March 13, 2026 11:59
@antmhs
Copy link
Contributor Author

antmhs commented Mar 13, 2026

Updated the comments and added the test for max_col. We are using both Java and Rust. Anyway, I added the new config to all bindings:

  • C and C++ use a struct for the config
  • C# uses a struct for the config
  • Go uses a struct for the config
  • Java uses a class with public fields for the config (I saw that's how it's done for other classes in the other bindings, but this is not very idiomatic Java. Ideally, you would have getters and maybe a builder)
  • Python uses keyword-only args for the config
  • Ruby uses a dict for the config (also, I fixed the Ruby build since it was failing after b4a69a1 which bumped serde_magnus from 0.9 to 0.11 which introduced a breaking change since deserialization now requires passing the Ruby object)
  • JS through WASM uses a JS object for the config (I also added a dependency on serde-wasm-bindgen for deserializing)

Copy link
Collaborator

@anakrish anakrish left a comment

Choose a reason for hiding this comment

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

@antmhs Thank you so much for taking care of bindings as well! Just couple more comments.

@antmhs antmhs force-pushed the custom-policy-limits branch from d495293 to 6d77a0a Compare March 13, 2026 14:59
@antmhs
Copy link
Contributor Author

antmhs commented Mar 13, 2026

Addressed the comments. Also, the workflow was failing because the column width was lowered to 200 in the bindings, now it's 2000 which should pass.

- Add PolicyLengthConfig struct with max_col, max_file_bytes, and
  max_lines fields, replacing hardcoded constants in the lexer.
- Add Engine::set_policy_length_config and clear_policy_length_config
  to allow callers to override the default limits.
- Add Source::from_contents_with_limits and from_file_with_limits for
  direct Source construction with custom limits; existing from_contents
  and from_file signatures are preserved using defaults.
- Add tests for default rejection, custom limits, and engine plumbing.
- Add bindings for C, C++, Python, WASM/JS, Java, Ruby, C#, Go
@antmhs antmhs force-pushed the custom-policy-limits branch from 6d77a0a to f14467f Compare March 13, 2026 15:31
@antmhs
Copy link
Contributor Author

antmhs commented Mar 13, 2026

Missed one config in bindings/c/main.c.

Copy link
Collaborator

@anakrish anakrish left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the contribution!

@anakrish anakrish merged commit 8986431 into microsoft:main Mar 13, 2026
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants