Conversation
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
imlk0
marked this pull request as ready for review
August 24, 2026 08:54
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
imlk0
marked this pull request as draft
August 25, 2026 02:14
|
@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>
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
imlk0
marked this pull request as ready for review
September 1, 2026 09:38
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
jialez0
reviewed
Sep 2, 2026
jialez0
left a comment
jialez0
left a comment
Collaborator
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(); |
Collaborator
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", |
Collaborator
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；或者让两个后端都能编译，再通过配置选择实际使用哪个。
Collaborator
Author
There was a problem hiding this comment.
已删除regorus-interpreter，仅保留regorus-regovm
| }) | ||
| } | ||
|
|
||
| /// Hoisted-cached strategy: the load-hoisted strategy plus a per-(policy_hash, |
Collaborator
There was a problem hiding this comment.
这个 benchmark 现在测的是一份单独复制出来的实现，而且缓存结构和生产代码还不一样，所以 benchmark 跑通也不代表生产缓存逻辑没问题，这次规则集合变化的问题就没有覆盖到。建议把缓存逻辑抽出来复用，或者直接通过 OPAInMemory::evaluate 测生产路径；性能测试和正确性测试也可以分开写。
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
imlk0
marked this pull request as draft
September 3, 2026 06:33
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
…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>
evaluate_with_regovm's program cache (keyed by policy_id) returned the cached rule map verbatim on a hash-matching hit. A later appraisal that asked for a rule absent from the first request's compilation would skip that rule as "not defined" — the policy defined it, but its program was never in the cache. Fix: on a hit, find requested rules missing from the cached map; if any, build the Engine once and compile only those, merge them into the map and persist. Full miss, partial miss and the all-cached fast path all flow through one hoisted Engine build. Two regression tests through the production OPAInMemory::evaluate path: - rule dimension: two appraisals of the same policy with disjoint rule sets, both directions, each must return the rule the policy defines. - data dimension: two appraisals of the same policy (cache hit) with distinct reference values (distinct `data`); the second must reflect its own `data`, proving the cached program does not bake in the first request's data document (regorus lowers `data.x` to `LoadData`, which reads the VM's runtime data store set per-evaluation via `set_data`). Co-Authored-By: Claude <noreply@anthropic.com>
The per-backend name-handling note documented validation behaviour that is now enforced in code: `build_extensions` validates each key with `is_valid_rego_extension_name` before it reaches generated source, and the interpreter path uses `Engine::add_extension` (no source interpolation). Drop the note rather than maintain prose that duplicates what the implementation already guarantees. Co-Authored-By: Claude <noreply@anthropic.com>
The regorus backend feature model was mutually-exclusive (regorus-interpreter XOR regorus-regovm, enforced by compile_error). That made several reasonable feature selections fail to compile: +regorus-regovm (with the default interpreter also on), --all-features, and --no-default-features (no backend at all). Make regorus-regovm the single opt-in backend and the interpreter the default compiled whenever regorus-regovm is off (cfg not(regorus-regovm) instead of feature(regorus-interpreter)). regorus-regovm takes precedence when enabled, so --all-features selects the VM path and --no-default-features keeps the interpreter. The regorus-interpreter feature is removed (it was a vestigial marker once the interpreter became the default-when-off); CI feature lists drop it accordingly. The wasm32 guard is retargeted to not(regorus-regovm): building wasm32 without regorus-regovm still errors (the interpreter needs a multi-threaded runtime), matching the existing wasm CI which passes --features ...,regorus-regovm. Verified: default, --no-default-features, +regorus-regovm, and --all-features each resolve to a single backend with no regorus conflict. (--all-features still fails on an unrelated, pre-existing tdx-dcap-ffi/tdx-dcap-rust mutex in the verifier crate, out of scope.) Co-Authored-By: Claude <noreply@anthropic.com>
…grams The compiled RVM program does not depend on the data document: regorus lowers `data.x` accesses to `LoadData` instructions that read the VM's runtime data store (set per-evaluation via `set_data`), so no data value is baked into the bytecode. `resolve_programs` therefore does not need the request's data_value at compile time — pass an empty object only to satisfy `compile_with_entrypoint`'s internal `prepare_for_eval`, which requires a valid (object) data document, and drop the now-unused `data_value` parameter. Evaluation still loads the real data per request via `vm.set_data`. - resolve_programs: drop `data_value` param; `add_data(Value::new_object())` - evaluate_with_regovm: drop the `&data_value` argument from the call Verified: regovm lib 56 pass (incl. evaluate_compiles_missing_rule_on_cached_policy); interpreter lib 42 pass; fmt + clippy --no-deps clean on both backends. Co-Authored-By: Claude <noreply@anthropic.com>
…ion path The PR #225 benchmark review (jialez0, comment on eval_bench) noted the bench tests a separately-copied cache impl structurally divergent from the production ProgramCache/resolve_programs, so bench-pass does not imply the production cache is correct — and at the time no non-bench test exercised the production cache path. ab2b56d fixed the rule-set-change bug and added one regression through OPAInMemory::evaluate; the remaining stale-source / hash-mismatch branch of resolve_programs was still untested. Add three production-path tests pinning every cache branch: - set_policy_drops_program_cache_slot (in_memory.rs): OPAInMemory::set_policy must eagerly remove() the policy_id's slot, not leave it for the hash check to evict lazily. White-box on the real ProgramCache. - evaluate_picks_up_external_policy_file_change (fs.rs): an external {policy_id}.rego overwrite (no set_policy -> no eager eviction) must be picked up by the next evaluate; under regorus-regovm this exercises resolve_programs' hash-mismatch branch directly through the fs read-fresh path. - common_evaluate_invalidates_cache_when_policy_source_changes (mod.rs): call common_evaluate with the same policy_id but different source; assert the cache auto-invalidates and the second appraisal reflects the new policy. Also derive Debug on CachedPolicy: OPA's #[derive(Debug)] (fs.rs) only compiles under the previously-untested fs+regorus-regovm feature combo (no CI matrix combines them) because CachedPolicy lacked Debug. Safe since regorus::rvm::Program already derives Debug; unblocks the fs+regovm build. Co-Authored-By: Claude <noreply@anthropic.com>
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
The interpreter doesn't need `arc` (regorus Rc types stay inside a sync spawn_blocking closure); unconditional `arc` taxed its AST churn ~12%. `arc` now pulls in only with `regorus-regovm` (the VM needs Send regorus types across the host-await future); the interpreter's ExtensionFunction future drops +Send (block_on'd locally, like the wasm variant). Bench (default EAR policy): interpreter 1.19->1.05ms (recovered to merge-base); regovm unchanged (2.09ms cold / 0.23ms warm). Co-Authored-By: Claude <noreply@anthropic.com>
|
@imlk0 ，您好，您的请求已接收，请耐心等待结果。 |
|
@imlk0 ，您好，未检测到有镜像需要构建，如需重新检测请评论 /start 。 |
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.