Skip to content

Switch OPA engine to RegoVM for real await function call - #225

Open
imlk0 wants to merge 4 commits into
mainfrom
regovm
Open

Switch OPA engine to RegoVM for real await function call#225
imlk0 wants to merge 4 commits into
mainfrom
regovm

Conversation

@imlk0

@imlk0 imlk0 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

This PR introduces two Cargo features to support different Regorus execution modes within our project:

  • regorus-interpreter: The legacy interpreter-mode policy engine.
  • regorus-regovm: The VM-based policy engine.

The latter (regovm) is a newer execution engine introduced in upstream (microsoft/regorus#730, microsoft/regorus#363) . It natively supports the__builtin_host_await instruction to invoke asynchronous extension functions, which allows for natural async function evaluation in upstream microsoft/regorus#667 and addresses asynchronous patterns discussed in microsoft/regorus#730. This capability is highly beneficial for single-threaded environments like WASM.
However, since regovm still has some gaps in its syntax support compared to the mature interpreter mode (as seen in ongoing fixes like microsoft/regorus#718), we provide both features to give users a choice. By default, regorus-interpreter remains enabled.

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@imlk0
imlk0 marked this pull request as ready for review August 24, 2026 08:54
@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@imlk0
imlk0 marked this pull request as ready for review August 25, 2026 03:55
@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@jialez0 jialez0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Few comments. 另外有个问题:我看现在Regorus 0.11的RVM编译还没有覆盖完整的Rego语法,虽然Trustee内置策略暂未使用这些语法,但是用户上传的自定义策略可能会受到这里更新的限制,这个语法支持情况是如何考虑的?

regovm_host_await_functions: &HashMap<String, RegoVmHostAwaitFunction>,
) -> String {
let mut ext = String::from(r#"# === trustee EXTENSIONS (generated) ==="#);
for key in regovm_host_await_functions.keys() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这里将调用方传入的函数名直接拼接进 Rego 源码和字符串字面量,没有标识符校验或转义。一个实际的攻击场景:带换行和注释的函数名成功注入了 allow := true,使原本 default allow := false 的策略返回 true。这里最好严格校验函数名为合法 Rego identifier,并正确转义字符串;更稳妥的方式是避免通过字符串拼接生成策略源码。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已添加检查函数is_valid_rego_extension_name(),并且在fn build_extensions()添加了检查

regorus::Value::from_json_str(&input).map_err(PolicyError::SetInputDataFailed)?;

let mut rules_result = std::collections::HashMap::new();
for rule in &evaluation_rules {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

现在在 evaluation_rules 循环内重复创建 Engine、加载相同 policy/data 并编译 RVM program。默认 EAR 一次求值包含多个 trust-vector rule,因此编译开销会被放大 N 次。我做了个测试,默认策略执行 20 次由基线约 0.67s 增至 4.92s,约慢 7.4 倍。可以考虑将 Engine 和模块加载移出循环,并考虑缓存编译结果或一次编译多个 entry point。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

我做了个mini bench,虽然没有复现出慢7.4倍,只复现出了慢4.83倍,但应该也可以做一个参考,结果如下表:

策略 干了啥 每次耗时 相对旧解释器
interpreter_baseline 旧的稳定后端,建一次引擎跑完所有规则 6.0 ms 1.00×
regovm_per_rule_rebuild(上次review的实现) 优化前的RegoVM:4条规则每条都重建引擎、重编译,编译被白跑4遍 29.0 ms 4.83×慢
regovm_load_hoisted(将Engine和模块加载移出循环) 引擎只建一次、策略只加载一次(不再每条规则重来) 11.3 ms 1.89×慢
regovm_hoisted_cached(当前实现) 上面基础上再加缓存:同一份策略编译过一次就记住,下次直接复用 2.4 ms 0.39×(比旧的还快)

目前当前pr已经针对regorus-regovmfeature实现最后一个优化策略,regorus-interpreter feature不受影响

Comment thread attestation-service/src/policy_engine/opa/fs.rs Outdated
Comment thread attestation-service/src/policy_engine/opa/fs.rs
The v0.1.0 tag of artifact-resolve-sdk does not build for
wasm32-unknown-unknown. Pin to rev e5ce1955 which carries the wasm
build fix, so the policy-artifact-server feature compiles on the wasm
target alongside policy-rvps.

Co-Authored-By: Claude <noreply@anthropic.com>
imlk0 added a commit to imlk0/trustee that referenced this pull request Aug 31, 2026
…naming

Add two mutually-exclusive Cargo features selecting the policy execution
backend, with compile_error guards enforcing exactly one is enabled (and a
wasm32 guard since the interpreter path needs a multi-threaded tokio runtime):

  - `regorus-interpreter` (default): the stable legacy path -- sync regorus
    `Engine` + `Extension`s driven via `tokio::task::spawn_blocking`, so the
    extensions' `block_on` never nests the runtime. Re-added from the
    pre-regovm code; the same async `ExtensionFunction` closures the regovm
    path drives through its suspend loop are reused unchanged via an
    `async_to_sync_extension` bridge that `block_on`s each closure's future
    on a blocking-pool thread.
  - `regorus-regovm`: the unstable Regorus VM suspendable host-call path
    (`__builtin_host_await` + `ExecutionMode::Suspendable`), needs no tokio
    runtime, works on wasm32.

The public interface is identical under either backend, so downstream code is
unaffected by the choice:
  - `RegoVmHostAwaitFunction`          -> `ExtensionFunction`
  - `with_extra_host_await_functions`  -> `with_extra_extension_functions`

The "host await" naming is dropped from our symbols/comments because the
scheme now spans two backends; only regorus's mandatory `__builtin_host_await`
builtin name remains in generated rego. `with_extra_extension_functions`
documents the caller's responsibility to ensure each key is a legal rego
function name (it is interpolated into generated rego source).

Dotted names (e.g. `crypto.sha256`) resolve on BOTH backends -- verified: the
`regorus-regovm` backend via generated function-rule wrappers
(`build_extensions`), the `regorus-interpreter` backend via regorus's
`add_extension` dotted-path resolution. The `crypto.sha256` end-to-end test
runs ungated on both.

All AS lib tests pass under each backend (interpreter: 62, regovm: 70).
rustfmt + clippy clean on the AS scope; the RVM-only `eventlog`
`manual_is_multiple_of` lint is the known local-1.96-vs-CI-1.88 drift.

Not pushed; PR openanolis#225 not updated.

Co-Authored-By: Claude <noreply@anthropic.com>
@imlk0
imlk0 marked this pull request as draft September 1, 2026 03:28
@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

imlk0 and others added 3 commits September 1, 2026 16:57
… feature-gating, and program-cache optimization

Switch the OPA policy engine from the legacy regorus interpreter path
(`spawn_blocking` + sync `Extension`s) to an opt-in RegoVM backend, while
keeping the interpreter as the default for stability.

Backend selection (mutually-exclusive Cargo features in
`attestation-service/Cargo.toml`, enforced by `compile_error!` guards in
`opa/mod.rs` — exactly one on; neither or both errors; interpreter
unavailable on single-threaded wasm32):
- `regorus-interpreter` (DEFAULT) — stable path: sync regorus `Engine` +
  `Extension`s via `tokio::task::spawn_blocking` (so the extension
  `block_on` never nests the runtime).
- `regorus-regovm` — unstable RVM suspendable host-call path
  (`__builtin_host_await` + `ExecutionMode::Suspendable`); works on wasm32.

Same async type bridges both backends. The public extension type is async
(`Arc<dyn Fn(Value) -> Pin<Box<dyn Future + Send>>>`, aliased
`ExtensionFunction`); the interpreter path wraps each async closure in a
sync `Extension` that `block_on`s on the blocking thread. So the same
`Vec<(String, ExtensionFunction)>` flows into either backend — zero change
at the call site. Dotted names (e.g. `crypto.sha256`) resolve on both
backends (regovm via generated function-rule wrappers, interpreter via
regorus's `add_extension` path resolution).

`OPAInMemory` gains a generic `with_extra_extension_functions` injection
point so a downstream crate can supply host functions regorus omits.

regorus's `rvm` feature (the RVM bytecode module) is forwarded only from
`regorus-regovm` (`regorus-regovm = ["regorus/rvm"]`), not from the
workspace baseline, so the interpreter build compiles no `regorus::rvm`
code. `arc` stays in the workspace baseline: both backends return
`regorus::Value` from `Send` async extension futures, so `arc` (making
`Value` `Send`) is required under either backend, not just `regorus-regovm`.

Performance: `evaluate_with_regovm` rebuilt the `Engine`, re-loaded the
same policy/data, and re-compiled an RVM program once per trust-vector rule
inside the `evaluation_rules` loop; a default EAR appraisal evaluates 4
rules, so compile cost was amplified ~4-5x locally, ~7.4x on slower
hardware. Two optimizations, both skip-safe (no multi-entry-point
compile, which would turn "missing rule -> skip" into "missing rule ->
whole compile fails"):
- hoist `Engine` + `add_policy`/`add_data`/wrapper load OUT of the rule
  loop (build once per cache-miss evaluation; per-rule
  `compile_with_entrypoint` still throws "not a valid rule path" -> caught
  -> skip, preserving the skip contract).
- cross-evaluation `ProgramCache` on `OPAInMemory` and fs `OPA`. Keyed by
  `policy_id` (NOT by content hash) with the policy content hash carried
  as a validation checksum: a lookup is a hit only when the id is present
  AND the stored hash matches the current source, so a changed policy
  source overwrites that entry in place. This bounds the cache to one
  entry per policy and evicts stale versions automatically — important for
  the fs-backed `OPA`, which reads the policy file fresh on every
  `evaluate` and would otherwise accumulate a never-evicted cache entry
  per content version when the file is overwritten on disk (unbounded
  memory growth). `RulePrograms = HashMap<String, Arc<regorus::rvm::Program>>`;
  the cached `Program` excludes per-eval `data`/`input` (set on the VM at
  run time) and the host-await wrapper is constant per engine instance.
  `set_policy`/`delete_policy` drop just the affected `policy_id`'s entry
  (`.remove`), not the whole cache.

The entire cache plumbing (`CachedPolicy`/`RulePrograms`/`ProgramCache`,
the `program_cache` field, construction, the `common_evaluate` param, the
call-site arg, the `.remove()`s) is cfg-gated to `regorus-regovm`: the
optimization is RVM-specific and the interpreter build carries zero
`regorus::rvm` types and no dead cache. `#[cfg]` on the `common_evaluate`
param keeps the signature clean under each backend — the param count ranges
6-8 (6 baseline, +1 with `policy-artifact-server`, +1 with `regorus-regovm`
for the program cache); the `#[allow(clippy::too_many_arguments)]` only
bites the regovm+artifact-server form (8).

The skip contract is pinned by `evaluate_skips_rules_not_defined_in_policy`
(a partial policy defining only some of the 4 trust-vector rules must skip
the missing ones, not error).

CI: the feature-gating made `regorus-interpreter`/`regorus-regovm` required,
so the `--no-default-features` jobs in `rust-check.yml` now specify a
backend (native jobs `+regorus-interpreter`; the wasm32 job `+regorus-regovm`
since the interpreter is unavailable on wasm32). A native regovm `cargo test`
job is added too — previously the regovm backend was only `cargo check`-ed
on wasm32 (compile-only, no tests run).

The `eval_bench` micro-benchmark is a separate follow-up commit.

Co-Authored-By: Claude <noreply@anthropic.com>
…che optimization

Adds `eval_bench`, a micro-benchmark that times the per-rule evaluation
path back-to-back in one process on identical inputs (empty `{}`
input against the default EAR policy; all paths agree on the 4
trust-vector rule values). Run with:

  cargo test -p attestation-service eval_bench_default_policy \
    -- --nocapture                  # dev profile
  ... --release ...                 # release numbers

The benchmark is gated to `all(test, feature = "policy-rvps", feature =
"regorus-regovm")`, so the default CI `cargo test` (which selects
`regorus-interpreter`) does not even compile it — it only runs under the
`regorus-regovm` backend (now covered by a native regovm `cargo test` job
in CI; previously regovm was only `cargo check`-ed on wasm32).

Four strategies are timed, ordered least- to most-optimized so each step
isolates one change:

- `interpreter_baseline`      — reconstructed origin/main regorus-interpreter
                                 path (one `Engine`, `eval_rule` loop).
- `regovm_per_rule_rebuild`   — the old per-rule rebuild/recompile path
                                 (reproduces the review finding; ~5x slower
                                 locally, ~7.4x on slower hardware).
- `regovm_load_hoisted`       — hoist `Engine`+policy/data load out of the
                                 rule loop; per-rule compile unchanged.
- `regovm_hoisted_cached`     — load-hoisted + cross-evaluation `ProgramCache`
                                 (the strategy production now ships).

`regovm_per_rule_rebuild_eval`/`regovm_load_hoisted_eval`/
`regovm_hoisted_cached_eval` are inlined copies of their strategy, NOT
calls into production `evaluate_with_regovm`, so the bench stays a stable
reference independent of the production refactor. They share a
`build_setup` helper (returning a 5-tuple factored into a `BuildSetup`
alias to satisfy `clippy::type_complexity` under `--tests`).

Co-Authored-By: Claude <noreply@anthropic.com>
…tion

`build_extensions` splices each caller-supplied extension key raw into a
Rego rule head (`{key}(arg) := v if { ... }`) and a string literal
(`"{key}"`). A key carrying a newline, comment, quote, or brace could
inject Rego source — appending `allow := true` would flip a
`default allow := false`. regorus ships no identifier validator for this,
so validate at the interpolation point itself.

Add `is_valid_rego_extension_name`: a dotted path of Rego identifiers
(each segment `[A-Za-z_][A-Za-z0-9_]*`, dot-separated, not a reserved
keyword). Dotted names are accepted because both backends resolve them —
rego.v1 admits a ref-headed function definition (regovm) and the Engine
resolves a dotted `add_extension` path (interpreter). `build_extensions`
now returns `Result` and rejects any name outside this set, so the gate
is unbypassable for the source-generation path.

Characterize the contract with non-ignored tests:
- regovm accepts a dotted wrapper name (compiles + end-to-end eval);
- interpreter accepts a dotted extension name via `add_extension`;
- the injection name is rejected before interpolation, keeping
  `default allow := false` intact.

On this branch `build_extensions`/`build_extensions_module` are
`#[cfg(feature = "regorus-regovm")]` (the interpreter path uses
`Engine::add_extension`, which does not interpolate source, so it is not
an injection surface). The validator and its helpers are gated to match,
so the interpreter lib build carries no dead code (CI clippy `-D warnings`).

Co-Authored-By: Claude <noreply@anthropic.com>
@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@imlk0
imlk0 marked this pull request as ready for review September 1, 2026 09:38
@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,您的请求已接收,请耐心等待结果。

@ostest-bot

Copy link
Copy Markdown

@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start

@jialez0 jialez0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

补充几条这次更新后的 Review 意见,主要是缓存正确性、feature 使用方式和测试覆盖。

// rule"; a skipped rule is simply absent from the cached map, so a later
// appraisal re-attempts it (cheaply).
let programs: RulePrograms = {
let cached = program_cache.read().await.get(&policy_id).cloned();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这里缓存命中只看了 policy_idpolicy_hash,但缓存里只有第一次 evaluation_rules 编译出来的规则。比如第一次请求 first,第二次请求 second,第二次会直接命中旧缓存,然后把 second 当成没定义跳过。我这边已经复现了,结果是 None,但策略里实际定义了 second := 2。建议缓存命中后再检查一下本次有没有新增规则,有的话补编译并合并到缓存里,同时加个规则集合变化的回归测试。

"fs",
"policy-rvps",
"policy-artifact-server",
"regorus-interpreter",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这里的 feature 设计可能不太好用:regorus-interpreter 是默认开启的,但又和 regorus-regovm 互斥。这样下游直接加 features = ["regorus-regovm"] 时,两个 feature 会一起打开,然后编译失败;--all-features 也会有同样的问题。另外,之前能用的 --no-default-features 现在也编不过了。建议只把 regorus-regovm 作为 opt-in feature,没开时默认走 interpreter;或者让两个后端都能编译,再通过配置选择实际使用哪个。

})
}

/// Hoisted-cached strategy: the load-hoisted strategy plus a per-(policy_hash,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这个 benchmark 现在测的是一份单独复制出来的实现,而且缓存结构和生产代码还不一样,所以 benchmark 跑通也不代表生产缓存逻辑没问题,这次规则集合变化的问题就没有覆盖到。建议把缓存逻辑抽出来复用,或者直接通过 OPAInMemory::evaluate 测生产路径;性能测试和正确性测试也可以分开写。

/// This is the generic extension point that lets a downstream crate supply
/// host functions regorus omits by design.
///
/// # Caller responsibility: name legality

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这里还写着 Trustee 不会校验扩展函数名,但现在 RVM 路径已经在 build_extensions() 里做校验了,建议把这段说明同步更新一下,避免调用方误解。

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.

3 participants