Style thing really but I think one that makes sense. I renamed only by
what was already the most popular option.
Tested building against Go 1.27 to make sure I didn't mess that up.
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
I don't even use it anywhere, but I end up in that code often enough
that I finally felt like I had to do something about it :)
This culminated in the side quest of implementing a builder for
ast.Object's which I think turned out quite well. There are many places
we could use that later, but this is already quite far out from where I
intended to be, lol
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
## Description
`sum` has an integer fast path that accumulates elements into a plain Go
`int`. Any element that fits a machine int takes this path, so a running
total that exceeds int64 wraps silently:
```rego
sum([9223372036854775807, 1]) # -9223372036854775808 (should be 9223372036854775808)
sum({9223372036854775807, 1, 2}) # -9223372036854775806 (should be 9223372036854775810)
```
`plus` is correct for the same values (`9223372036854775807 + 1` is
`9223372036854775808`), so `sum` and `+` disagree.
#8887 (fixes#6281) added exact `big.Int` accumulation for elements that
are individually larger than 64 bits, but that fallback only runs when
an element does not fit a machine int (`n.Int()` fails). When every
element fits int64 and only the running total overflows, the fast path
is still taken and wraps.
## Fix
Guard the fast-path addition and fall back to the existing
`exactIntAccumulate` big.Int path on overflow, the accumulator `product`
already uses. Small-int, float, mixed, and >64-bit-element inputs are
unchanged.
## Test
Extended
`v1/test/cases/testdata/v1/aggregates/test-aggregates-bignum.yaml` with
a case where each element fits int64 but the sum does not: array and set
overflow, negative overflow, an at-limit value that must stay on the
fast path, and a `+` control. Results are rendered with `sprintf`
because the golden-case loader parses expected numbers as float64. The
case fails on `main` and passes with this change. Added the matching
WASM exception (#3711), as #8887 did, since the result exceeds 64 bits.
`go test ./v1/topdown/` passes.
Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
Fixes: #3680
Built-ins require ground operands. When a Var, Ref, or comprehension
reaches one anyway, it returns a generic eval_type_error, which gets
collected into builtinErrors and turned into undefined unless strict
built-in errors are enabled -- so bugs in OPA surface as "this rule
didn't match". #3681 was this shape: a captured function output went
untracked in the save set during partial evaluation, and a variable
reached count().
Check the plugged operands first and return an internal error naming the
offending term. The check runs before the builtin-call timer starts, so
the error path needs no stopTimer. The captured output operand is
exempt: walk() is legitimately called with a non-ground composite there.
The check is shallow -- a type switch plus IsGround, a field read on
composites -- because deep-walking every operand would make
constant-time built-ins linear; benchmarks are unchanged. A nested but
ground term such as [data.foo] is therefore not detected.
---------
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
When in the position of a `not` operand, the `{ ... | ... }` expression
is ambiguous. Did the author mean to negate a set comprehension or did
they intend a not-body containing a set union? Both interpretations are
close to nonsensical, as they both negate an always truthful statement,
but since even nonsensical code is expected to work, this PR makes this
construct illegal at parse-time.
The author can use parens to specify whether they intend a not-body or a
value operand. E.g.:
* `not ({ ... | ... })` is a set comprehension
* `not {(... | ...)}` is a not-body containing a set union
**Note:** This PR covers edge cases unlikely to affect many users, but
we should nevertheless not produce or consume ambiguous code; especially
since the end-user doesn't have explicit control over e.g. PE output,
which can produce unorthodox Rego at times.
---------
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
### Why the changes in this PR are needed?
The OCI bundle and discovery download paths used a deterministic default
OCI store under the process temp directory when `persistence_directory`
was
not configured. In a shared temp directory, that made the store location
predictable before OPA initialized the local OCI layout.
This PR follows up on a report discussed with the OPA maintainers, where
Anders Eknert confirmed this can be handled as a regular public issue/PR
because it assumes local access to the system.
### What are the changes in this PR?
This changes the non-persistent default OCI store handling so OPA no
longer
uses a fixed shared temp path for bundle/discovery OCI downloads. When
callers
do not provide a store path, `download.NewOCI` now creates a private
temporary
OCI store directory.
The existing `persistence_directory` behavior is preserved: when
configured,
OPA still stores OCI state below `<persistence_directory>/oci`.
This also adds regression coverage for the default and explicit OCI
store path
behavior.
### Notes to assist PR review:
Tested with:
```bash
GOMODCACHE=/ssd1/CCS/recurbug/verifies/build/go-mod-cache GOCACHE=/tmp/opa-gocache /tmp/opa-go/go/bin/go test ./v1/download ./v1/plugins/bundle ./v1/plugins/discovery
```
### Further comments:
I kept this PR focused on the default non-persistent OCI store path and
did
not include unrelated bundle or discovery refactoring.
---------
Signed-off-by: kimdu0 <dino700072@gmail.com>
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Co-authored-by: kimdu0 <dino700072@gmail.com>
Co-authored-by: Sebastian Spaink <sebastianspaink@gmail.com>
The existing uribuiltins cases cover the happy paths only. These pin
five inputs where net/url.Parse rejects or normalises in ways a non-Go
reimplementation is likely to get wrong: ASCII %-escapes in the host,
invalid userinfo, non-ASCII digits in the port, raw_path collapsing to
the decoded path when it round-trips under the default encoding, and a
bare "]" being valid in a reg-name host.
All five diverged in the Java SDK (open-policy-agent/java-opa-sdk#156)
while passing the existing fixtures. Expected values were taken from OPA
v1.19.0; per-case rationale is in the fixture comments.
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Fixes#8897
When a reference has a composite subject (e.g. [1, 2][i]) or a dynamic
index term (e.g. [1, 2][input.x]), the compiler hoists that part of the
ref into a generated local. Previously these locals leaked verbatim into
type errors, e.g.:
undefined ref: __localq0__[i][j]
The type checker's var rewriter only knew how to map user variables back
(via RewrittenVars), so anonymous generated locals were rendered as-is.
Record a mapping from each generated ref-subject/dynamic-operand local
back to the original term value (in localVarGenerator.subjects) and use
it when rendering refs in type errors, so the original expression is
shown instead:
undefined ref: [1, 2][i][j]
---------
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Didn't know this was a thing now. That certainly helps! Also some
follow-up fixes from the previous modernize PR.
---------
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Signed-off-by: Charlie Egan <charlie_egan@apple.com>
Co-authored-by: Charlie Egan <charlie_egan@apple.com>
We do this in the hottest spots already, but might as well do it
wherever we can. Had to deploy a few tricks in some places, but I think
the code is better off too.
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
`opa fmt` isn't idempotent for rules it can't write on a single line:
the first pass runs the rule together with the one that follows, and
only a second pass inserts the missing blank line.
Given this policy:
```rego
package example
allow if { {"admin", "dev"} }
deny if input.blocked
```
`opa fmt` produces output that it doesn't consider formatted:
```rego
package example
allow if {
{"admin", "dev"}
}deny if input.blocked
```
```console
$ opa fmt --fail example.rego; echo $?
2
```
Running it a second time inserts the blank line and settles:
```rego
package example
allow if {
{"admin", "dev"}
}
deny if input.blocked
```
with this fix fmt goes to the second result immediately
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
The "strcov.Atoi" mention was both odd and incorrect. Hopefully a little
better now. Also a few other minor things.
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Mostly automated fixes from running:
```
go run golang.org/x/tools/go/analysis/passes/modernize/cmd/modernize@latest --fix ./...
```
But carefully reviewed, and several fixes reverted as they looked like
they potentially could be less performant, and in a few cases due to
bugs in the analyzer that changed semantics of the code. Will report
these upstream.
Mostly good fixes though!
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
## Description
`json.match_schema()` treated `{"enum": []}` as if the `enum` keyword
were absent, so every instance validated successfully (strings, numbers,
null, booleans, arrays, objects).
Per the [JSON Schema validation
spec](https://json-schema.org/draft/2020-12/json-schema-validation#section-6.1.2),
`enum` succeeds only when the instance deep-equals one of the listed
values. An empty list therefore has nothing to match and must make the
schema **unsatisfiable** (fail for every instance).
### Root cause
In vendored `internal/gojsonschema`, enum presence was only checked via
`len(enum) > 0`. A present empty `enum` array left a nil/empty slice and
skipped validation entirely.
### Fix
- When the `enum` keyword is present, store a non-nil slice (possibly
empty).
- When the keyword is absent, leave `enum` as `nil`.
- Validate whenever `enum != nil`, so empty enum always produces an enum
error.
### Breaking change
Schemas with `"enum": []` previously matched **any** value; they now
reject **every** value.
This is intentional and aligns with the JSON Schema spec. Accidental
empty allow-lists (e.g. schema generation from a zero-entry allow-list)
now fail closed rather than fail open.
No change for:
- missing `enum` keyword (still accepts any value)
- non-empty `enum` (same accept/reject behavior as before)
### Tests
- `internal/gojsonschema`: `TestEmptyEnumUnsatisfiable` (unit)
- `v1/topdown`: `json.match_schema` cases for empty and non-empty enum
Fixes#8910
## Checklist
- [x] I have read the [contribution
guidelines](https://www.openpolicyagent.org/docs/latest/contributing/).
- [x] I have added/updated tests for my change.
- [x] All new/updated code follows the existing conventions of the
affected area.
---------
Signed-off-by: Dean Chen <862469039@qq.com>
### Why the changes in this PR are needed?
`x % y` raises a spurious `modulo by zero` error whenever `y` is a
nonzero multiple of 2^64:
```rego
10 % 18446744073709551616 # 2^64 -> error "modulo by zero", want 10
5 % 55340232221128654848 # 3*2^64 -> error, want 5
7 % 340282366920938463463374607431768211456 # 2^128 -> error, want 7
100 % -18446744073709551616 # -2^64 -> error, want 100
```
`arithRem` checks for a zero divisor with `b.Int64() == 0`.
`big.Int.Int64()` returns the low 64 bits when the value does not fit in
an int64, and those bits are zero for any multiple of 2^64, so a clearly
nonzero divisor is read as zero.
Big-integer modulo itself is already correct — `10 %
18446744073709551617` (2^64+1) returns 10 today — so this is the zero
check misfiring, not the modulo semantics that #8887 deliberately left
out of scope.
### What are the changes in this PR?
- `arithRem` tests `b.Sign() == 0` instead of `b.Int64() == 0`. `Sign()`
is zero only for an actual zero, so genuine `x % 0` still errors and
nonzero divisors of any magnitude go through `big.Int.Rem`.
- A golden case in `test-arithmetic-bignum.yaml` covering 2^64, a
multiple of 2^64, 2^128, a negative multiple of 2^64, and the 2^64+1
control that already passed. It fails on `main` and passes with this
change.
- A WASM exception for the new case, matching the existing >64-bit
arithmetic cases, since WASM cannot represent integers larger than 64
bits (#3711).
### Notes to assist PR review:
`go test ./v1/topdown/` passes. The divide path is unaffected:
`arithDivide` operates on a `big.Float` and already guards with `acc ==
big.Exact && i == 0`, so `10 / 18446744073709551616` does not hit the
same issue.
Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
After a rule head, an expression that parses to a set term is discarded
and re-parsed as `{ BODY }`, so that `p if { true }` is a body and not a
one-element set. That reinterpretation is only correct when the
expression starts with a `{`. `p if not {...}`, `p if set()` and `p if
({1, 2})` all parse to a set, but none have a leading brace, so there is
no ambiguity with a rule body and they should be kept as a sets.
Note: `not { ... }` with `future.keywords.not` imported parses to an
explicit body, not a set term, and so was never affected.
---------
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
The set union infix operator shares the `|` symbol with comprehensions,
creating edge-cases where the formatter would change the semantics of a
set union into a comprehension.
E.g. When formatted, `{or(a, b)}` becomes `{a | b}`; where the former is
a set containing the union of two sets, and the latter is a set
comprehension.
Note: the `or()` built-in backing the `|` set union infix operator isn't
advertised through documentation, and it's very unlikely that anyone is
affected by this issue. Nevertheless, formatting shouldn't change the
semantics of a policy.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Follow-up to #8900. Moves the gzip encoding and decoding config
validation off the Go `validateAndInjectDefaults` methods and onto
embedded Rego policies, injecting defaults and reporting value errors.
Each config registers its recognized options via
`config.RegisterConfigSpec` so unknown-option warnings live with the
owning struct. Field type validation stays in the Go decode step.
---------
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Before this fix, e.g., the rule:
```rego
p if { {false} }
```
would be formated to:
```rego
p if {false}
```
which changes the semantics from a rule body containing a single set
(`{false}`) to a rule body containing a single scalar value (`false`).
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
### Why are the changes in this PR needed?
The DAP debugger currently truncates variable values to a hardcoded
limit of 100 characters, with no way for callers to configure it.
Long values cannot be inspected or copied whole from a debugger UI.
### What are the changes in this PR?
- Add a new `SetMaxVariableLength` Debugger option (in both
`v1/debug` and the top-level `debug` package)
- A value of 0 disables truncation; the default stays 100 characters
for backward compatibility
- Plumb the limit through `variableManager` and `namedVar`, so it
applies to top-level and nested (object/array/set) variables alike
- Relax `truncatedString` to return the original string unchanged
when the limit is <= 0
- Add `TestTruncatedString` and `TestVariableValueLengthLimit`
covering the default limit, unlimited (0), negative, custom limits,
and boundary cases
### Notes
- `go build ./...` and `go test ./v1/debug/...` both pass
- Default behavior is unchanged; the new option is opt-in
- This PR was developed with AI assistance (Claude Code)
### Further comments
Refs #8890
---------
Signed-off-by: summy wu <summy.wu81@gmail.com>
Makes OPA build and pass its tests on Go 1.27, while keeping Go 1.25 and
1.26 working. JSON output is unchanged on every supported version.
Go 1.27 json package honours `encoding.TextAppender`. Many v1 ast types
implement AppendText to build their Rego string cheaply, so on 1.27 they
would have marshalled as Rego text. Files built only with 1.27 now
implement MarshalJSONTo.
Library users should keep using `json.Marshal` etc. The MarshalJSONTo
methods are implementation details, are absent from 1.25 and 1.26
builds, and may change.
---------
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Signed-off-by: Charlie Egan <charlie_egan@apple.com>
Co-authored-by: Charlie Egan <charlie_egan@apple.com>
Fixing an issue where the `future.keywords.not` import would erroneously
import other future keywords.
E.g. consider the following v0 module:
```rego
package example
import future.keywords.not
p if {
not input.x
}
```
The `future.keywords.not` import also imports the `if` keyword. This fix
makes the above module invalid.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
### Why the changes in this PR are needed?
Fixes#8894.
`opa fmt` expands a one-line `if` condition into a block whenever the
rule head's *value* expression spans multiple lines, even though the
condition itself is a single simple term. For example:
```rego
foo := sprintf(
"%d",
[1],
) if allow
```
was reformatted to:
```rego
foo := sprintf(
"%d",
[1],
) if {
allow
}
```
### What are the changes in this PR?
The inline-`if` path in `writeRule` decides whether to keep `if <term>`
on one line by comparing the body term's row to the rule head's row:
```go
if rule.Body[0].Location.Row == rule.Head.Location.Row {
```
`rule.Head.Location.Row` is the head's **start** row. Once the head
value wraps onto later lines, the single body term sits on a later row
than the head start, the equality fails, and formatting falls through to
the block form.
The fix compares against the head's **end** row instead (start row plus
the number of newlines in the head's location text), so a single body
term on the same line as `if` stays inline regardless of how many lines
the head value occupies. Single-line heads are unaffected (end row ==
start row), and genuinely multi-statement bodies still expand as before
(they don't hit the `len(rule.Body) == 1` branch).
### Notes to assist PR review:
Added `v1/format/testfiles/v1/test_issue_8894.rego` (+`.formatted`) with
the exact repro from the issue; it fails on `master` (expands to a
block) and passes with this change. The rest of the format golden suite
is unchanged.
Signed-off-by: Kunalbehbud <b.kunal2002@gmail.com>
### Why the changes in this PR are needed?
Policies often need only the first or last few parts of a split string,
but `split()` always returns every part. The common workarounds add
noise:
```rego
[name, email, _, _] := split(user, ";")
[name, email] := array.slice(split(user, ";"), 0, 2)
```
### What are the changes in this PR?
Adds `strings.split_n(x, delimiter, n)`:
- Positive n: returns the first n parts from the left
- Negative n: returns the last abs(n) parts from the right
- n=0: returns an empty array
- If abs(n) exceeds the number of parts, all parts are returned
### Notes to assist PR review:
Semantics follow the design agreed on in the review of #8361.
### Further comments:
Fixes#8344
This change was developed with AI assistance.
Signed-off-by: wonju lee <wonju@kia.com>
Co-authored-by: wonju lee <wonju@kia.com>
## Summary
- add a concrete JSON unmarshal path for tester.Result errors
- preserve the existing marshaled output byte-for-byte
- reconstruct structured topdown errors from opa test --format json
output
Fixes#8014
## Testing
- go test -count=1 ./v1/tester/... ./tester/...
- go test -count=1 ./cmd/...
- go build ./...
- go vet ./v1/tester/... ./tester/... ./cmd/...
- gofmt check on the changed Go files
Signed-off-by: Victor Solano <victor.solanonunez@gmail.com>
Field names in the SQL emitted by the Compile API come from partially
evaluated refs, so a policy selecting a dynamic key such as
input.fruits[input.column] puts caller-controlled text in an identifier
position. That text was emitted verbatim, which turns
WHERE fruit.name = 'allowed'
into
WHERE fruit.name = 'allowed' OR 1=1 -- = 'allowed'
and an application appending the filter to its query returns rows the
policy denies.
Quote field segments that are not bare identifiers at the UCAST-to-SQL
boundary, escaping any embedded quote character. Ordinary column names
stay unquoted, so existing filters keep their current shape and remain
case-insensitive on Postgres.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
Partial evaluation can produce a binding where a variable is equated to
a call that references itself, e.g. `bt & ut == bt` yields
`bt = and(bt, ut)`. Copy propagation would substitute the variable with
that binding, reintroducing it on the RHS to form a self-referential
term, which sent the following ast.Transform into unbounded recursion
and overflowed the stack, crashing the server on /v1/compile.
Skip substituting a binding that mentions the variable, so circular
calls are left in place instead of expanded.
Fixes: #6428
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
I was wondering what this really bought us. It's not much.
NB this is still a rule set shaped to _reward_ ordering (shared
high-selectivity gates + unique details). On a homogeneous or
low-selectivity rule set the gap collapses toward nothing.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Creating the scalars map for each node was expensive, and would in many
cases sit unused. Now we initialize it only before use, which shaves off
almost a million allocations from `regal lint bundle`, and I imagine is
quite a boost for evaluation of many other policy types.
Also:
- Add benchmarks for the indexer that I used to try things out
- Avoid a few heap allocations by avoiding interface boxing to Value
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Co-authored-by: Stephan Renatus <stephan.renatus@gmail.com>
Submitting some miscellaneous changes I had locally. A few allocs saved,
but mostly style fixes here, like simplifying known var/var equality
using `==` and so on. Nothing controversial, or so I'd like to think :)
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Switching the REPL to reeflective/readline (#8882) dropped liner's
fallback to a plain reader for non-terminal stdin. This made `opa run`
with piped/redirected stdin spin at 100% CPU instead of exiting, and
made the v1/runtime REPL tests flaky via leaked spinning goroutines.
Loop now uses the readline editor only for a real terminal and a plain
line reader (stops cleanly at EOF) otherwise, configurable via
WithConsoleInput / Params.ConsoleInput.
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Fixes: #962
Pasting a snippet containing a tab into the REPL triggered
tab-completion on the pasted tab, corrupting the input (e.g. injecting a
completion candidate mid-line and producing a spurious parse error).
Spaces were unaffected, so the bug only surfaced with tab-indented
pastes.
The fix is bracketed paste: with it enabled, a terminal wraps pasted
text in paste markers and the line-reader inserts it literally instead
of treating an embedded tab as a completion request. The previous
reader, peterh/liner, has no bracketed-paste support and is unmaintained
(last release 2021; the same fix was proposed upstream in
peterh/liner#114 in 2019 with no traction), so this replaces it with
reeflective/readline, which supports bracketed paste, completion, and
history.
OPA's existing multi-line buffering (the r.buffer parse-retry mechanism)
is kept as-is; readline's native multi-line editing is left disabled to
avoid changing REPL behavior.
---------
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Fixes: #8757
The pooled metadataParser reuses its comment slice across METADATA
blocks, and Parse stored it on the Annotations without copying. A later
block's parse then overwrote an earlier annotation's comments,
corrupting its EndLoc.
opa build --optimize=1 prunes comments by each annotation's row range,
so a corrupted EndLoc dropped a METADATA block's body while keeping the
bare "# METADATA" marker, yielding bundles that fail to parse on load.
Clone the slice so each Annotations owns its comments.
---------
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Building the rule index for `<ref> in <collection>` panicked with
"illegal value" whenever the collection contained an object or set
element (or an array nesting one), since updateMemberRefInValue
inserts each collection element into the trie as-is, without
restricting it to scalars/arrays like the equality-based indexing
does. Such elements now fall back to the trie's "any" node, like an
unbound Var: the rule stays a candidate for every input, and body
evaluation determines the actual result.
Other Value types considered (Ref, comprehensions, Call) can't
actually reach the trie from compiled Rego, since the compiler
rewrites them into separate statements before the index is built;
verified this individually against `opa eval`, so the panic remains
for them as a genuine invariant check.
Fixes#8918.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
The compile-time fail hints used a fuzzy match against the ref's
top-level segment to suggest typo fixes (e.g. input.frut -> input.fruit).
levenshtein.ClosestStrings returns the exact match itself when the
top-level segment already matches a declared unknown, so any failure
caused by a missing/undefined sub-field (rather than a misspelled
top-level name) produced a hint suggesting the exact same ref back,
e.g. "input.resource.heading undefined, did you mean
input.resource.heading?".
Skip the hint entirely in that case, since the fuzzy matcher has no
visibility into sub-fields and suggesting the ref unchanged is a no-op.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
The `test.WithTempFS` helper is used _extensively_ throughout our tests.
Since `t.TempDir()` became a thing (Go 1.16), it probably shouldn't be,
as that function does all the same things but in a more idiomatic
manner.
Main issues with `test.WithTempFS`:
- It doesn't take a `*testing.T`, making failures reported without
correct/helpful location.
- It creates a new scope for no particular reason, where it could just
have returned the root directory instead. An additional scope == an
additionl level of indentation.
This change adds the new `test.TempDir` and `test.TempDirOf` functions,
which tries to address these issues. There are way too many places where
`test.WithTempFS` is used for me to fix in a single PR, so more will
have to come later. Most of the changes here don't even use the new
functions, but replace the use of `test.WithTempFS` with `t.TempDir()`
directly, as no files were passed to the function there.
Also:
- Replace a number of `reflect.DeepEqual` calls with better alternatives
(not using reflection)
Recommended reviewing with whitespace diffs hidden!
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
### Why the changes in this PR are needed?
The OPA SDK (`v1/sdk`) doesn't currently expose the rego-layer
`EvalHTTPRoundTripper` primitive added in #7180. Go library embedders
that call `opa.Decision(...)` therefore have no way to observe or shape
`http.send` round-trips from their policies.
Concrete use cases:
- **Observability**: attach an outbound HTTP round-trip decorator so
each policy-triggered `http.send` shows up in the embedder's
request-scoped telemetry (spans, structured logs, per-plugin round-trip
capture, etc.).
- **Credential injection**: mint or refresh a caller-supplied bearer /
mTLS certificate for the exact scope of one decision — a variation on
the Minder pattern that motivated #7180 at the rego layer.
- **Middleware**: transparently thread retry, backoff, or rate-limit
policies in front of `http.send` without policy authors having to do
anything.
Today, doing any of the above requires either forking the SDK, calling
into `rego`/`topdown` directly (losing the SDK's plugin/bundle wiring),
or globally registering an `HTTPTracingService` via
`pkg/tracing.RegisterHTTPTracing` (which is global, per-process, and
coarse-grained).
### What are the changes in this PR?
Adds `DecisionOptions.HTTPRoundTripper` (type
`topdown.CustomizeRoundTripper`) and threads it through `evalArgs` into
the `pq.Eval(...)` call via the existing `rego.EvalHTTPRoundTripper`.
- **Backward-compatible.** New nil-defaulted field; existing callers see
identical behavior. Covered by a nil-passthrough subtest.
- **Per-request scope.** Threading happens on `pq.Eval` opts, not
`rego.New` opts, so the prepared-query cache remains shared across
decisions and each `Decision` can supply its own transform.
- **API consistency.** Uses the same `topdown.CustomizeRoundTripper`
type introduced in #7180 for the rego layer — no new naming to litigate,
and the SDK field name (`HTTPRoundTripper`) matches
`rego.EvalHTTPRoundTripper`.
- **Doc-commented gotcha.** The doc comment calls out that the received
`*http.Transport` may be `nil` for plain-HTTP requests (post-#7927),
matching the pattern exercised by
`topdown/http_test.go:TestHTTPWithCustomTransport`'s
`secretTransport.Transform`.
### Notes to assist PR review:
- Zero changes to public types beyond one new nil-defaulted field on
`sdk.DecisionOptions` (which is re-exported to `sdk` as a type alias —
no shim edits needed).
- No changes to `PartialOptions` — `http.send` is non-deterministic and
deferred during partial evaluation, so plumbing it there would have no
runtime effect. Happy to add for symmetry if reviewers prefer.
- New test `TestDecisionWithHTTPRoundTripper` covers (a) nil field is a
no-op (regression guard) and (b) a caller-supplied
`CustomizeRoundTripper` is invoked exactly once per `Decision` and its
wrapped transport receives the request.
### Further comments:
Related prior work:
- #7180 — rego-layer `EvalHTTPRoundTripper`; this PR is the SDK-layer
follow-up.
- #5967 — `plugins.WithDistributedTracingOpts` (per-instance HTTP
wrapping for bundle fetches / decision-log pushes; addresses a different
traffic class — background rather than per-Decision — and is
intentionally out of scope here).
- #7927 — nuance about `*http.Transport` sometimes being nil, referenced
in the new doc comment.
Signed-off-by: Paulo Costa <eu@paulo.costa.nom.br>
## Description
Fixes#6281.
`plus`, `minus`, `multiply`, `sum` and `product` corrupt integers that
need more than 64 bits of precision. They route the operands through
`builtins.NumberToFloat` (a `big.Float` carrying the default mantissa),
so the value is rounded before the operation is applied:
```rego
18446744073709551617 + 1 # 18446744073709551616 (should be ...618)
18446744073709551617 * 2 # 36893488147419103230 (should be ...234)
sum([18446744073709551617, 1]) # 18446744073709551616 (should be ...618)
product([18446744073709551617, 2]) # 36893488147419103230 (should be ...234)
```
This is the same defect class that #8857 fixed for `format_int`.
There is a root cause underneath it that is worth calling out
separately, because it is the reason this fails silently rather than
erroring:
```go
func NumberToInt(n ast.Number) (*big.Int, error) {
f := NumberToFloat(n) // rounds here
r, accuracy := f.Int(nil)
if accuracy != big.Exact { // cannot fire: the rounded float IS an exact integer
return nil, errors.New("illegal value")
}
return r, nil
}
```
The accuracy check exists to catch inexact conversions, but the rounding
has already happened inside `NumberToFloat`, and the rounded value is
itself an integer, so the check passes. `NumberToInt` returns the wrong
`big.Int` and reports no error:
```
NumberToInt(18446744073709551617) = 18446744073709551616
NumberToInt(123456789012345678901234567890) = 123456789012345678899921813504
```
`NumberToInt` also backs `BigIntOperand`, so the corruption is reachable
from the `bits.*` builtins as well.
## Fix
- `NumberToInt` parses integer literals exactly with `big.Int`, and
falls back to `big.Rat` for fractional and exponent forms, so `1e30`
stays exact and a genuinely fractional value is rejected rather than
silently truncated.
- `plus`/`minus`/`multiply` apply the operation on exact `big.Int`s when
both operands are integers, mirroring how `format_int` was fixed.
- `sum`/`product` accumulate on `big.Int` when every element is an
integer, and fall back to the existing float accumulator otherwise
(which also preserves the existing operand-type errors).
Float, mixed int/float, and small-int behaviour is unchanged, including
the existing interned-small-int fast paths. `0.1 + 0.2` still yields
`0.3`.
Division and modulo are untouched. The thread raises open questions
about their expected semantics for big integers, so they felt out of
scope here.
## Test
Two golden cases: `arithmetic/bignum exact` and `aggregates/bignum
exact`, covering >2^64 values through each operation, a 30-digit value,
negatives, and the small-int / float / mixed cases that must not change.
Results are rendered with `sprintf` in the golden cases because the case
loader parses expected numbers as `float64`, which cannot represent
these values. Asserting on the numbers directly fails on the loader
rather than on the builtin. `format_int`'s golden case sidesteps the
same problem by returning strings.
Both cases fail on `main` and pass with this change. `go test
./v1/topdown/` passes with no regressions; the failing tests I do see on
Windows (`TestCertSelectionLogic`, and several in `v1/rego`) fail
identically on a clean checkout of `main`, so they are pre-existing and
unrelated.
Added WASM exceptions for both cases, as #8857 did, since WASM cannot
represent integers larger than 64 bits (#3711).
---------
Signed-off-by: Atishyy27 <atishayjain2704@gmail.com>
Co-authored-by: Atishyy27 <atishayjain2704@gmail.com>
Following #8891, which moved top-level config validation to an embedded
Rego policy, this migrates the `server/metrics` and `metrics_export`
configs onto Rego as well. Plugins register their recognized options via
`config.RegisterConfigSpec` (derived from their struct fields) so
unknown-option warnings live with each struct that brings the config.
The goal is migrate more `validateAndInjectDefaults` in follow up PRs,
this setups the foundation for other migrations to follow.
---------
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Part of #2745
Like most of his ideas, @anderseknert's suggestion to use Rego to
replace the `validateAndInjectDefaults` functions throughout the
codebase is another winner.
This PR starts the migration by replacing the top-level
`validateAndInjectDefaults` in `v1/config/config.go` with an embedded
policy, `validate.rego`. The policy injects the top-level defaults
(`default_decision`, `default_authorization_decision`, `labels`) and
reports unrecognized configuration options, so a typo such as
`decision_log` instead of `decision_logs` is logged as a warning at
startup rather than silently ignored.
It's evaluated in `ParseConfig` using the low-level `ast`/`topdown`
packages rather than the top-level `rego` package. This keeps `config`
off the heavy `rego → bundle → …` dependency web (which would otherwise
create import cycles as more packages' tests reach `config`), and we
don't need any of the `rego` package's conveniences here — it's one
module compiled once and a single query. The Rego unit tests run in CI
via `build/run-rego-tests.sh` (and locally with `make rego-test`).
This sets the foundation for the other plugin
`validateAndInjectDefaults` functions to migrate to Rego as well; where
the logic isn't too complicated it should be a fairly easy replacement.
At the moment all known keys live in `validate.rego` under `_specs` to
support the "warn on unrecognized options" check, but the
plugin-specific entries can move closer to each plugin as it migrates.
It would also be nice for `_specs` to be auto-generated somehow in the
future.
Supporting extension of config validation with custom policies is
something I'd like to follow up with, so keeping #2745 open for now.
I also think these policies could be reusable with
[java-opa-sdk](https://github.com/open-policy-agent/java-opa-sdk) 👀
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
CogeneratedExprs built its result slice by iterating the 'visited' map,
so the order of the returned expressions was randomized by Go's map
iteration. PrettyEvent walks these co-generated expressions in order to
populate the --var-values output; when a base variable is reachable from
refs in more than one co-generated expression (e.g. 'tc' in
'tc.data == tc.expected.data'), the walk order decides which column that
variable is reported at. The randomized order made the pretty output
flip between runs, causing TestFailVarValues to fail intermittently.
Append to the result slice during visitation instead, using the map
only for dedup, so the returned order follows the deterministic
generates/generatedFrom structure.
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Fixes: #3546
`:=` is currently sugar for `=`, so the safety checker can satisfy an
assignment's RHS by unifying backwards through the LHS. As a result `x
:= y; x = 7` compiles (binding `y` to `7`) even though `y` is never
assigned, and `x := y; obj[x]` can silently turn a constant-time lookup
into full iteration.
Mark equality expressions rewritten from `:=` and exclude the LHS from
the safe basis when computing their output variables, so a value cannot
flow LHS->RHS. RHS reference iteration (e.g. `some k; v := obj[k]`) is
unaffected.
This is a deliberate semantic change, and is backwards-incompatible:
affected policies now fail with a rego_unsafe_var_error.
---------
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
Fixes#7830
When a failing test expression selected a value out of a local variable
via a ground ref (e.g. tc.data), the pretty --var-values output only
showed the base variable's full value rather than the selected value
being compared. Resolution previously only worked for refs the compiler
rewrites into locals (calls, arithmetic, variable-keyed refs); plain
ground object selections stay inline and were never resolved.
Handle ast.Ref terms in PrettyEvent's var walker: resolve a ground ref
whose base is a bound local against the local bindings and report the
selected value at the ref's location, while still descending to report
the base variable too.
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
SliceStack is a plain generic LIFO stack. GroupStack builds on it for
the two-level 'stack of slices' pattern shared by functionMocksStack and
saveStack: whole groups are pushed/popped, while elements are pushed/popped
onto the top group only.
Both stacks zero vacated slots on pop, at both levels, so popped values
(ast terms, bindings) aren't kept alive by the backing arrays.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
eval.go had two nearly-identical sync.Pool wrappers (evfp, evbp) whose only
job was to zero out evalFunc/evalBuiltin fields before returning them to the
pool, so pooled values don't keep terms/bindings from a prior call alive.
Add util.ResettablePool[T, PT], a generic pool for types whose pointer
implements Reset(). The reset method is resolved via a compile-time pointer
constraint (PT resettable[T]) rather than a runtime interface assertion, so
there's no extra dispatch cost on the Put hot path compared to the
hand-written version.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Anders Eknert