v2.0.0 — security hardening + language completeness - #75
Merged
Conversation
… 11 crates, stdlib 1.0-stable)
…harden VM (2.0 tranche) Remediation of confirmed findings from the codebase security research (claudedocs/research/boruna/). This tranche ships the gated security + arithmetic-correctness fixes; evidence signing, coordinator ownership model, and the language type-system buildout are staged (see REMEDIATION-PLAN.md). Security: - SSRF (crates/llmvm/src/http_handler.rs): resolve every DNS address and reject private/reserved targets, strip IPv6 brackets so literal checks fire, and follow redirects manually with per-hop re-validation (was: literal-IP-only + unvalidated auto-redirects). - Content-addressing (coordinator handle_complete): reject a worker output_hash that doesn't match SHA-256(output_json) before it reaches the store + audit chain (+ regression test). - Coordinator fail-closed: refuse to start on a non-loopback bind without auth unless BORUNA_COORD_ALLOW_INSECURE=1 (was: warn + serve). - Stored XSS (evidence serve): HTML-escape all bundle-derived output (+ tests). - Path traversal: reject '..'/'.' in storage ref_to_run_id (s3/gcs/azure) and in the MCP template-apply name. Correctness (2.0 breaking): - Checked i64 Add/Sub/Mul/Div/Mod/Neg -> VmError::ArithmeticOverflow (was: debug-panic / release silent-wrap, which broke determinism; also fixes the i64::MIN/-1 panic past the div-zero guard). - Actor: a crafted SpawnActor with an out-of-range function index now fails the actor instead of panicking the scheduler process (was: .expect() -> DoS). Docs: correct version/count drift (v1.9.0, 33 builtins, 12 MCP tools, 11 crates, all 13 stdlib packages 1.0-stable, stability.md heading contradiction). Includes the full research knowledge base under claudedocs/research/boruna/. Gates: cargo test --workspace --features boruna-cli/serve, cargo fmt --check, cargo clippy --workspace --all-targets --features boruna-cli/serve,boruna-vm/http -- -D warnings — all green.
Closes the detection half of the #1 High finding (plaintext evidence bundles were forgeable because verify was purely self-referential over a rewritable manifest — F1/F2, verify-A CONFIRMED). - verify_bundle_with_opts(): recompute + check manifest.bundle_hash (internal consistency), plus an operator-supplied out-of-band expected_bundle_hash ANCHOR and a require_encryption flag (blocks the downgrade-to-plaintext strip). - CLI: `evidence verify --expected-bundle-hash <hex>` and `--require-encryption`. - The anchor is the real fix: recomputing the hash alone proves nothing (an attacker recomputes it too) — test_verify_anchor_detects_forged_manifest forges a fully self-consistent manifest that plain verify accepts but the anchor rejects. - verify_bundle / verify_bundle_with_kek retained as thin wrappers. Full ed25519 manifest signing (removes the out-of-band-anchor requirement) remains staged — see claudedocs/research/boruna/REMEDIATION-PLAN.md. Gates: cargo test --workspace --features boruna-cli/serve (48 suites, 0 failed), cargo fmt --check, cargo clippy ... --features boruna-cli/serve,boruna-vm/http -- -D warnings — all green.
Any authenticated worker could complete/fail/extend ANOTHER worker's in-flight step: the terminal CAS keyed only on (run_id, step_id, claim_id), and claim_id is a predictable per-step counter from 1 (verify-B CONFIRMED, highest-value distributed finding). - persistence: add RunCheckpointStore::step_claimed_by(run_id, step_id) -> the worker_id holding the row's lease. - coordinator: reject_if_not_claim_owner() gates handle_complete/fail/extend_lease at the trust boundary (under the store lock, so read+CAS are atomic in-process), returning 403 coord.claim_not_owned when the caller is not the claim holder. - tests: complete_by_non_owner_returns_403 (new); complete_with_stale_claim_returns_409 rewritten so the SAME worker reclaims (still exercises the lease-expiry CAS). Boundary-check approach (matching the S12 content-addressing fix) avoids threading worker_id through the shared persistence CAS + ~22 test call sites. S9 (approval-gate seizure) remains to do. Gates: cargo test -p boruna-cli --features serve (green; one pre-existing dashboard HTTP-timing flake confirmed passing in isolation), plus workspace gate to follow.
Wipe DEK and KEK bytes from memory so freed heap/stack pages don't retain key material: - Envelope now impls Drop, zeroizing its DEK. - unwrap() zeroizes the decrypted DEK Vec the AEAD returns. - resolve_kek() zeroizes the BORUNA_BUNDLE_KEK env hex String. rotate.rs clones the (non-secret) info since Envelope is no longer movable-out. Adds zeroize dep to orchestrator.
[S1] Return a compile error instead of silently truncating list/record sizes, call arity, and function arity when they exceed u8::MAX (255). Wrapping the count corrupted the operand stack at runtime. [G11] Pop the trailing bare expression of a while body so the operand stack stays balanced each iteration; previously one value leaked per iteration and grew unbounded until stack overflow.
/api/runs/{id}/approve took no token and was first-writer-wins, so any bearer or
worker-cert holder could seize AND pre-empt a human-in-the-loop approval gate
(verify-B S9 + N1). Root cause: the ApprovalGate arm minted no token at all
(unlike ExternalTrigger).
- runner: the ApprovalGate open path now mints a per-gate token via
acquire_trigger_token (stashed in metadata.triggers, payload empty), mirroring
ExternalTrigger. New `approval_gate_token(store, run_id, step_id)` read helper.
- coordinator: handle_approve_run requires the token, constant-time compared to
the stashed one; 403 coord.approval_token_invalid on missing/mismatch. Unknown
run / not-a-gate falls through to the precise 404/state error (no masking).
- CLI: `workflow approve|reject --token <hex>`; send_approve_remote forwards it.
- tests: approve_run_rejects_wrong_token (new, +token-less), and the existing
advances/double-decision/integration tests fetch the stashed token.
This completes finding F (S6 landed earlier). Boundary approach (like S6/S12) —
record_approval_decision_in_store and its ~10 tests are untouched.
Gates: orchestrator --lib 454/0, boruna-cli --features serve unit 152/0 +
approve integration test green, fmt clean.
The `!self.capabilities.is_empty() &&` guard made an EMPTY capability set allow EVERY effect (fail-open). Because a malformed/empty explicit policies() lands on PolicySet::default() (empty capabilities), an app could ship an empty or malformed policies() and silently get allow-all. Remove the short-circuit so an effect whose capability is not in the declared set is DENIED even when the set is empty. Explicit empty/malformed policies() now fails closed. Scope: does NOT touch the runtime.rs "no policies() function at all -> allow_all()" convenience (lines 81-86); that documented default for apps that omit policies() entirely is unchanged. Only EXPLICIT empty/malformed policies() now denies. Tests: renamed test_policy_empty_capabilities_allows_all -> test_policy_empty_capabilities_denies_all and flipped its assertion to is_err(); test_policy_default_is_restrictive now actually asserts the default policy DENIES an effect (previously only checked is_empty(), which under the old fail-open code was a no-op guarantee — the name lied); test_policy_batch_limit_exact_boundary now declares net.fetch so it still exercises the effect-count boundary rather than tripping the new capability deny. Self-review (policy-trust change — account-takeover / privilege-escalation pass): - Every EffectKind maps to a capability_name() (net.fetch, db.query, fs.read, fs.write, time.now, random, actor.spawn, ui.render, llm.call, actor.send). No effect kind bypasses the check; the deny now covers all of them when the cap is absent, including empty sets. - check_batch: limit check first, then check_effect per effect. Empty caps now deny every effect; empty effects list still passes (no capability used). No bypass in the compose. - send() and send_with_executor both route through send()->check_batch, so the executor path is gated too. HostEffectExecutor's own allow_all gateway is downstream of this authoritative gate; unchanged. - from_value: non-Record -> default() (empty caps -> deny); Record with missing/wrong capabilities field -> extract_string_list -> empty caps -> deny. Malformed policies() reaches the deny path. Fail-closed holds. - Change is strictly a privilege REDUCTION (fail-open -> fail-closed); the no-policies() allow_all escalation path is untouched. No new vector.
…tions
G7: parse and codegen `for x in list { ... }` — desugars to an index
loop over a List reusing ListLen/ListGet opcodes.
G8: parse `Map<K, V>` and `Fn(A, B) -> R` type annotations into the
existing TypeExpr::Map/Fn variants.
G1: emit parsed-but-dead `ensures` clauses — bind the return value to a
local `result`, assert each postcondition (Op::Assert), then return it.
Propagates the new Stmt::For variant through typeck and the exhaustive
Stmt matches in boruna-tooling and boruna-cli.
… staged for reconciliation
Indirect call targets (a function passed as a value, or any computed callee) were silently dispatched to function #0 — codegen hardcoded Op::Call(0, argc) in the fallback path, so every higher-order call ran the wrong function (frontend finding G2). - bytecode: new Op::CallIndirect(u8) (byte tag 0x14) — callee FnRef on top of the stack, N args below. - vm: CallIndirect pops the FnRef, dispatches to its func_idx (TypeError if the callee is not a function reference). - codegen: the indirect-call path now pushes the callee expr and emits CallIndirect instead of the hardcoded Call(0, ..). - test: apply(double, 21) == 42 through a Fn(Int)->Int parameter. Gates: bytecode/vm/compiler green (+ new test), workspace clippy -D warnings = 0.
Closes the "sign-side" of verify-A finding #1: a signed evidence bundle proves integrity via the operator's own key, removing the need to carry an out-of-band bundle_hash anchor. Additive and forward-compatible — existing unsigned 1.0 bundles verify unchanged (format_version stays "1.0"): - BundleManifest gains an OPTIONAL `signature: Option<ManifestSignature>` ({ algorithm, public_key hex, signature hex }) with skip_serializing_if = "Option::is_none". The unsigned manifest bytes (and thus bundle_hash) are byte-for-byte unchanged. - EvidenceBundleBuilder::with_signing_key(&[u8;32]) signs the finalized bundle_hash bytes (added AFTER hash computation, never feeds the hash). - verify_bundle_with_opts(dir, VerifyOptions{kek, trusted_pubkey, require_signature}). When a signature is present it is verified over bundle_hash, bundle_hash is recomputed to bind actual manifest content, and (when pinned) the signer key must equal trusted_pubkey — the actual trust root. Errors: evidence.signature_invalid / _untrusted_key / _required. verify_bundle / verify_bundle_with_kek preserved as wrappers. - CLI `evidence verify` gains --verify-key <hex> and --require-signature, mirroring the existing --bundle-encryption-key wiring. - rotate::compute_bundle_hash clears signature to match finalize. ed25519-dalek 2 added default-features=false (std, zeroize) per ADR 001. 6 new verify tests: signed passes, file tamper, bundle_hash tamper, wrong pinned key, require-signature on unsigned, unsigned still passes.
…m-match + typeck remain
Enums could be declared and pattern-matched but never CONSTRUCTED — there was no expression form for a user enum value, so `pattern_to_tag` returning -1 for every EnumVariant arm was dead code (every enum match collapsed to the first arm). This completes the feature end to end: - lexer: new `::` token - ast/parser: `Enum::Variant` / `Enum::Variant(payload)` construction expr - codegen: emit MakeEnum(type_id, variant_idx); resolve_enum_variant() maps the qualified name to (type_id, index); pattern_to_tag is now a method that resolves a variant name to its declaration index (VM Op::Match already dispatches on Value::Enum.variant, so no VM change) - typeck + tooling (format printer, diagnostics walkers, serve tag collector): handle the new Expr variant Additive, non-breaking (`::` was never a valid token before). +2 e2e tests. Known limitation: duplicate variant names across enums resolve to the first match in patterns (VM matches on variant index, ignoring type_id).
The type checker did name resolution only. It now rejects a direct call to a named function with the wrong argument count (CompileError::Type). Calls whose callee is a local binding (first-class function value / higher-order param) are skipped — their arity isn't known at this layer, and Op::CallIndirect handles them at runtime. Non-breaking: the existing .ax corpus (stdlib libs, example workflows, framework apps) all type-check and run correctly, so none carry arity mismatches. Verified green across compiler/vm/tooling/framework/orchestrator. Deeper strict checking (type inference, match exhaustiveness, record-field typing) remains a deferred, intentionally LTS-breaking follow-up: it rejects existing loose programs and needs a corpus-migration + strictness decision.
…rict type inference remains
…arnings) First step toward strict typing, per the chosen warn-only rollout: a new analyzer pass surfaces type mismatches as Severity::Warning (E009) via `lang check` / `boruna_check` without blocking compilation or execution. Two checks, both inference-free and conservative: - let bindings: `let x: T = <expr>` where the initializer's concrete type != T - call arguments: a direct call to a user function whose argument's concrete type != the declared parameter type (skipped when the callee is a local/param first-class function value) The local type environment only reasons about types it can name with confidence — literals, annotated bindings, record/enum constructors, and user function return types. Generics (Option/Result/List/Map/Fn), builtins and binary ops stay untyped, so a mismatch is reported only when BOTH sides resolve to differing concrete names. Verified zero false positives across all 64 corpus .ax files (examples + libs). +3 tests. Deferred to a later strict pass: type inference through binary ops, match exhaustiveness as types (already covered separately as E005), and promoting these warnings to hard errors at 2.0.
Security-hardening + language-completeness major. Remediates the whole-codebase research audit (3 High, several Medium, language gaps). Breaking: integer overflow is now a runtime error; coordinator and framework defaults fail closed (each with a documented override). See CHANGELOG 2.0.0.
|
Bench compare Threshold for regression: ≥ 10% slower mean.
|
…message The SSRF hardening renamed the literal-IP block message to 'blocked request to private/reserved IP', but two http-feature tests still asserted the old 'private IP' substring. These tests only run under `--features http` (a separate CI step), so per-crate gating didn't exercise them. Behavior is unchanged — the private IP is still blocked; only the assertion text was stale.
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
1 participant
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.
First major release. Remediates every finding from a whole-codebase research audit (3 High, several Medium, plus the "statically typed but unchecked" language gaps).
Security
403 coord.claim_not_owned)403 coord.approval_token_invalid)evidence serveLanguage
Enum::Variant(payload)+ real per-variant match tagsOp::CallIndirect)forloops,Map<K,V>/Fntypes,ensurespostconditionsBreaking changes (hence 2.0.0)
VmError::ArithmeticOverflow)BORUNA_COORD_ALLOW_INSECURE=1)output_hash!=output_jsonpolicies()now denies (was allow-all)All crates gated green individually (compiler/vm/tooling/framework/orchestrator/cli), fmt + clippy clean. Full-workspace validation runs in CI here.
See the
[2.0.0]entry in CHANGELOG.md for detail + migrations.