Conversation
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
jialez0
left a comment
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
这里将调用方传入的函数名直接拼接进 Rego 源码和字符串字面量,没有标识符校验或转义。一个实际的攻击场景:带换行和注释的函数名成功注入了 allow := true,使原本 default allow := false 的策略返回 true。这里最好严格校验函数名为合法 Rego identifier,并正确转义字符串;更稳妥的方式是避免通过字符串拼接生成策略源码。
There was a problem hiding this comment.
已添加检查函数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 { |
There was a problem hiding this comment.
现在在 evaluation_rules 循环内重复创建 Engine、加载相同 policy/data 并编译 RVM program。默认 EAR 一次求值包含多个 trust-vector rule,因此编译开销会被放大 N 次。我做了个测试,默认策略执行 20 次由基线约 0.67s 增至 4.92s,约慢 7.4 倍。可以考虑将 Engine 和模块加载移出循环,并考虑缓存编译结果或一次编译多个 entry point。
There was a problem hiding this comment.
我做了个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不受影响
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>
…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 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
… 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>
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
|
@imlk0 ,您好,您的请求已接收,请耐心等待结果。 |
|
@imlk0 ,您好,未检测到有镜像需要构建,如需重新检测请评论 /start 。 |
jialez0
left a comment
There was a problem hiding this comment.
补充几条这次更新后的 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(); |
There was a problem hiding this comment.
这里缓存命中只看了 policy_id 和 policy_hash,但缓存里只有第一次 evaluation_rules 编译出来的规则。比如第一次请求 first,第二次请求 second,第二次会直接命中旧缓存,然后把 second 当成没定义跳过。我这边已经复现了,结果是 None,但策略里实际定义了 second := 2。建议缓存命中后再检查一下本次有没有新增规则,有的话补编译并合并到缓存里,同时加个规则集合变化的回归测试。
| "fs", | ||
| "policy-rvps", | ||
| "policy-artifact-server", | ||
| "regorus-interpreter", |
There was a problem hiding this comment.
这里的 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, |
There was a problem hiding this comment.
这个 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 |
There was a problem hiding this comment.
这里还写着 Trustee 不会校验扩展函数名,但现在 RVM 路径已经在 build_extensions() 里做校验了,建议把这段说明同步更新一下,避免调用方误解。
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_awaitinstruction 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-interpreterremains enabled.