And enable more staticcheck linters. I saw staticcheck failures
mentioned in another PR, so thought I'd check it out.
- `WriteString(fmt.Sprintf)` -> `fmt.Fprintf`
- Rewrite calls to deprecated `*Rule.Path()`
- Don't use `==` to compare `time.Time`
- Use inline ignores over config exclusions of paths
- Remove 'varcheck' ignores as no longer used
- Remove v0 topdown/graphql.go (!)
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
* wasm: update wabt and binaryen in builder image
* wasm: bump ubuntu and llvm
* wasm: bump LLVM 13 -> 21, adjust headers
* wasm: make docker optional
We depend on it in our builds, but if you happen to bring
clang (LLVM 21)
clang++ (LLVM 21)
wasm-ld (LLVM 21)
wasm2wat (wabt)
wasm-opt (binaryen)
node
you should be able to build the opa.wasm blob without the docker image.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Originally meant to be `array.concat_n`, but this name is better
as the behavior of this function differs from `array.concat` —
namely that `array.flatten` accepts any type of valued in the
input array. Only arrays are however flattened, and the rest
are appended directly to the flattened output.
Note that this function only flattens at the topmost level of
the input array — not recursively! A cursory look
at a few other languages suggest a single level is the common case.
But if others feel we should flstten more, I'm happy to make an update.
The C code for a Wasm implementstion here is cowboy coded, and
I did not manage to run the tests on my machine due to some
`docker` <-> `container` differences. I mostly just imitated
the existing code in the array category. I doubt it'll work
on the first try, but only CI can judge me.
Also:
- Remove `opa fmt` step from the Rego CI step, as this is done by
Regal anyway a little later in the list of tasks.
- Replace some hard-coded `docker` names in the `Makefile` with `$(DOCKER)`
- Added name of built-in function missing to the unsupportedBuiltinErr
error, as it has happened a few times now that I've used `:=` in a
query, and had no clue what built-in it referred to.
Fixes#8226
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Adding string interpolation support to the Rego language.
An interpolated string is composed of a template-string that can contain zero or more template-expressions that interpolates values into the string generated at eval-time.
Requires the `template_strings` capability feature and `internal.template_string` built-in function.
Implements: #4733
- Bump golangci-lint -> 2.6.2
- Fix all `deprecatedComment` "notices should be in a dedicated paragraph, separated from the rest" reports
- Enable `appendCombine` and fix all "appendCombine: can combine chain of X appends into one" notices
- Enable `preferFprint` and fix the few reported issues
- Fix various issues reported only once or twice, like `zeroByteRepeat`
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Brace yourselves! For there are many touched files here. No changes
in semantics however.
Spent a long time trying out the various optional rules gocritic
provides, and settled for a few of them. There are more I really
like, but that would take many hours to address across the codebase.
Perhaps others find gocritic too pedantic? If so, we can merge the
fixes without enabling the rule.
Signed-off-by: Anders Eknert <anders@styra.com>
And many smaller performance improvements. The indexer recycling results
is one of the most impactful performance improvements as of yet, and alone
saves more than 2 million allocations in the Regal lint benchmark. The indexer
is also more efficient, as `values` are no longer stored on the struct. Thanks
@tsandall for that code!
Also included a bunch of small improvements from my perf branches.
**Before**
```
1209043041 ns/op 3255157224 B/op 64026192 allocs/op
```
**After**
```
1197131792 ns/op 3194124864 B/op 61876276 allocs/op
```
Signed-off-by: Anders Eknert <anders@styra.com>
And update code to conform to the rule.
- Replace unnecessary fmt.Sprintf with string concatenation
- Replace fmt.Sprint with more efficient strconv.Itoa
- Replace static fmt.Errorf calls with more efficient errors.New
Thanks @srenatus for pushing me down this rabbit hole!
Signed-off-by: Anders Eknert <anders@styra.com>
All packages, except for `cmd` and `internal`, have been moved into a new `v1` root package.
Old packages are kept for backwards-compatibility reasons. All contained code is replaced with simple type aliases and proxy functions to `v1` implementations.
Old packages default to the Rego v0 syntax, new `v1` packages default to the Rego v1 syntax.
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
In the Wasm modules built with OPA, arithmetic comparison for very large numbers
could be wrong, caused by an integer overflow.
Fixes#6991.
Signed-off-by: Pierre Troger <pierre@narval.xyz>
Fixing possible memory leak where caches grow uncontrollably when large amounts of regexes or globs are generated or originate from the input document.
Fixes: #6828
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Fixing an issue where a non-collection `every`-domain didn’t fail evaluation.
Removing a possible attack surface, where an attacker with the ability to craft portions of the input document could replace a value with an expected collection type, that is known to be processed by an `every`-statement, with a non-collection value and thereby would cause the policy to accept a query that should otherwise be rejected.
Fixes: #6762
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
When feeding a `char *` into `re->Match()`, it was converted to a StringPiece,
taking its size as `strlen()`. For our (long) input, that wasn't resulting in
the correct size, and did then freak out the re2 match input validation if the
regular expression has an end anchor, but the endpos wasn't the same as its
length. Since the endpos was taken from `s->len`, and the "length" taken via
the mentioned StringPiece's strlen() call, they did indeed not match.
Worked around by feeding it a properly-constructed std::string instead. I'm a
C++ novice at best, but it does the trick, and I'm reasonable certain it's less
wrong than before.
Fixes#6376.
Signed-off-by: Stephan Renatus <stephan@styra.com>
Basically lifting this compiler optiimization for the Wasm compiler into the planning stage: If we already know at plan time that a certain (in)equality check fails/succeeds, we don't need to do it. The less work, the better.
* planner: don't emit `NotEqualStmt{A: ..., B: false}` where superfluous
* planner: don't emit `EqualStmt{A: x, B: x}`
for string and bool constants
* compiler/wasm: remove optimizations
Signed-off-by: Stephan Renatus <stephan@styra.com>
The schema of the input document for the authorization
policy is known to OPA. This feature leverages that
to perform automatic type checking on the authorization policy.
The checks happen on policies provided to OPA on start-up and
also those provided via bundles. This check is enabled by default
and can be disabled using the `--skip-known-schema-check` flag
on `opa run`. This feature will help catch errors such as
typos, mismatch types etc. in these policies and provide precise
feedback to the policy author.
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
This patch removes ordered block storage in fixed-sized block freelists
in the OPA WASM memory allocator. Variable-sized block allocation still
orders blocks so that free() can coalesce them back into larger sized
blocks. This greatly reduces the runtime of opa_free() for fixed-size
blocks as it turns it from an O(N) operation to an O(1) operation.
This comes at the cost that reducing the heap_ptr implicitly on
opa_free() becomes impractical since reduction will stop at the first
fixed-sied block regardless of whether it is allocated or not. In
practice, what this means is that the allocator can never combine
fixed-size and variable-sized blocks. However, it was rarely able to do
so previously: only when the two blocks happened to be free at the same
time and line up with the heap_ptr.
This patch also adds support for a new function called opa_free_bulk() that
enables releasing memory objects always in O(1) time per object and
O(N log N) worst case for releasing N objects. The patch works by
freeing variable-sized objects (which would normally take O(N) time per
free) to a temporary holding list and setting a flag indicating that the
next variable-sized allocation needs to merge said holding list.
When releasing the holding list, the memory allocator first merge-sorts
in address-order the released blocks and then merges and coalesces them
into the variable-sized block list in address order. This takes at most
O(max(M+N, N log N)) time where M is the number of blocks on the
variable freelist and N is the number of blocks bulk freed.
The patch also updates the __opa_value_free() function to take a new
parameter named 'bulk' which directs the function passes to its various
type-specific subroutines. Every time one of the type-sepcific
subroutines goes to free an object it invokes either opa_free() or
opa_free_bulk() depending upon the 'bulk' parameter. (This is
abstracted by a function __opa_free_maybe_bulk() in value.c)
Calls to opa_value_free() or opa_value_free_shallow(), will set the
the 'bulk' parameter to false preserving the existing behavior.
However, the opa_value_add_path() and opa_value_remove_path()
functions will invoke the function with 'bulk' set to true to ensure
that the cascaded free operations on objects each take only O(1) time.
Finally, the patch re-enables the RESTAuthzAllow100Paths benchmark.
Fixes: #5901
Signed-off-by: Chris Telfer <chris.telfer@sophos.com>
This commit fixes several memory leaks in the WASM engine that occur
when a caller mixes incremental calls to opa_value_path_add() /
opa_value_path_remove() with actual policy evaluations. The issue
occurs due to a combination of lack of deep free of internal data
structures and the fact that eval() and opa_eval() calls reset the heap
to free temporary memory that they previous allocated.
More details about the issues and their fix are described in detail at
https://github.com/open-policy-agent/opa/issues/5785.
The changes in this patch fall into 5 categories:
1. Adding support for both internal WASM functions and external WASM
callers to perform a "deep" free of OPA values by freeing not only
the immediate object memory but all the opa values it refers to.
The opa_value_free() function now does this by default and is
also an exported function. The opa_value_free_shallow() is added
for the few cases where shallow frees are required, primarily in
eval()-invoked functions.
2. Enable stashing of free blocks prior to eval() and opa_eval() calls.
Eval calls will always leak free blocks due to the way that
opa_heap_ptr_get() works. This patch adds three new exports allowing
the user to save this memory from leaking.
* opa_heap_blocks_stash() -- saves free heap blocks to shadow
freelists.
* opa_heap_blocks_restore() -- restores the allocated heap blocks from
shadow freelists.
* opa_heap_stash_clear() -- discard any saved heap blocks on the shadow
freelists. (this is used for resetting VM heap to an initial state)
3. Update the WASM calling conventions in the SDK. This includes using
the new APIs to avoid leaking memory when adding or removing data
from the data doc. It requires bumping the WASM ABI to 1.3
4. Adding unit tests for the WASM ABI 1.3 functions.
5. Adding documenttion for the WASM ABI 1.3 functions.
Fixes: #5785
Signed-off-by: Chris Telfer <chris.telfer@sophos.com>
With the introduction of ref heads in #4660, the planned IR
still mostly worked, but it was bypassing the CallDynamic
optimization when it shouldn't have.
This commit re-works some of the rule planning to more robustly
handle ref heads.
Also adds a few test cases to get a grip on what should and
should not happen.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
The `object.keys` function will return a set of all top-level keys on
a given object. Since object keys in Rego don't have the same
restrictions as names in JSON name-value pairs, we also ensure
support for non-string key types.
Fixes#5363.
Signed-off-by: Kevin Swiber <kswiber@gmail.com>
Got a few warnings from my IDE about redundant type conversions,
so I decided to look into it. Added the unconvert linter to our
checks, and fixed the violations. Added two ignore comments as I
wasn't sure about whether they'd change the semantics of the code.
Signed-off-by: Anders Eknert <anders@eknert.com>
This has been semi-public anyways: people depend on the JSON structure to be
kept as-is.
So we might as well make the structs public, and make working with this easier
from golang. No need to copy the struct definitions manually.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit adds the `prealloc` linter to the list of linters for OPA, and fixes up the miscellaneous locations in the code that the linter found where we could easily preallocate slices.
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
With the embed directive, we no longer need our custom code that predates
Go 1.16. Also, with the release of 1.19, we no longer desire compatibility
with anything predating 1.16, so this cleanup becomes possible.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Using null for delimiters disables delimiters in glob matching. Preferable over regex on some cases for performance reasons.
Fixes#4923.
Signed-off-by: vinhph0906 <vinhph0906@gmail.com>
Co-authored-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit adds support for named argument declarations for built-in
functions as well as additional metadata/annotations on built-in
functions (e.g., descriptions, categories, etc.) This commit allows us
to generate a data file (builtin_metadata.json) that other tools can
consume to improve the Rego authoring experience.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Co-authored-by: Torin Sandall <torinsandall@gmail.com>
This package is deprecated, archived, and in maintenance mode, since Go
errors support wrapping natively.
For #2152.
Signed-off-by: Jason Hall <jason@chainguard.dev>
Due to excessive nesting of blocks, the break statement wasn't breaking
out of enough of them: this would this add an element to the result set
when it should have turned into undefined.
This was the case in two conditions:
a. the dynamic lookup not finding a data function to call, and the resulting
static data lookup failing to resolve the ref; or
b. the dynamically called function returns undefined
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit adds support for serializing/deserializing plans into/from
JSON. This allows us to compile policies out into JSON so that they
can be transpiled or interpreted in other environments.
To support these changes we have implemented custom marshaling on the
Block type and Operand (previously called LocalOrConst) type.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
wasm: Add support for WASM and simple tests.
internal: Add opa_json_is_valid to map of wasm built-ins.
docs: Indicate that WASM support is now available for json.is_valid.
Fixes#4140
Signed-off-by: Kristian Svalland <kristian.svalland@gmail.com>
The function `array.reverse` takes an array as an argument, and returns an array with a reversed order of elements.
The function `strings.reverse` takes a string as an argument, and returns a string with a reversed order of unicode code points.
WASM support is included for both built-ins.
Fixes#3736
Signed-off-by: Kristian Svalland <kristian.svalland@gmail.com>
Anders Eknert