Skip to content

[CTX-1004] fix(lua,runtime): tighten bitty.env key bound to ADR 0006 (#1751) - #1752

Merged
Xuepoo merged 1 commit into
mainfrom
ctx-1004/fix-env-key-adr0006
Oct 7, 2026
Merged

Xuepoo merged 1 commit into
mainfrom
ctx-1004/fix-env-key-adr0006

Conversation

@Xuepoo

@Xuepoo Xuepoo commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Priority: P1 | Area: area:lua,area:security,area:runtime | Labels: bug,P1,area:lua,area:security,area:runtime | Milestone: v0.1.0 | RFC: OQ-031 | Task: CTX-1004

Closes #1751.

Implements #1751 per bitty-docs#381 Option 1 (tighten host; ADR 0006 stays normative; see bitty-docs PR #444).

Change

  • crates/bitty-lua/src/host.rs: ENV_KEY_MAX_BYTES 128 -> 64; env_key_shape_ok to ^[A-Z_][A-Z0-9_]*$ 1..64 (uppercase + digits only); validate_env_key emits E_ENV_KEY_INVALID (validation class) for empty / over-bound / shape failures before any grant check; updated bridge docs + unit tests (max-length accept, lowercase/mixed-case reject, grant shape/authorizes pins).
  • crates/bitty-lua/tests/lua_parity.rs: env_bridge_rejects_malformed_keys_before_grants now expects E_ENV_KEY_INVALID for empty/shape/mixed-case/65-byte keys; non-string argument stays E_DEF_INVALID.
  • crates/bitty-runtime/src/plugin_runtime/services.rs: env_key_shape_rejected_before_grants expects E_ENV_KEY_INVALID, adds lowercase/mixed-case cases.
  • crates/bitty-runtime/src/plugin_runtime/mod.rs: env_grant_keys doc now states the ADR 0006 ^[A-Z_][A-Z0-9_]*$ prefix bound.
  • specifications/unsafe-ffi-audit.md: no env-key rows exist (only std::env::set_var test row); no update needed.

Acceptance

Evidence

  • cargo fmt --all -- --check PASS.
  • cargo clippy --workspace --all-targets -- -D warnings PASS.
  • cargo check --target x86_64-pc-windows-gnu -p bitty-lua -p bitty-runtime --all-targets PASS (no platform-conditional code touched; cross-check anyway).
  • cargo test -p bitty-lua --lib PASS (94 passed).
  • cargo test -p bitty-lua --test lua_parity PASS (17 passed).
  • cargo test -p bitty-lua --test host_bridge PASS (39 passed).
  • cargo test -p bitty-runtime --lib plugin_runtime PASS (222 passed).

Do NOT merge: leaving open for review.

Summary by CodeRabbit

  • Behavior Changes
    • Environment variable keys are now limited to 64 characters and must use uppercase letters, digits, and underscores, beginning with an uppercase letter or underscore.
    • Empty, malformed, lowercase, mixed-case, and over-length keys are rejected with a consistent invalid-key error before grant checks. Non-string keys continue to return the existing invalid-definition error.

@Xuepoo Xuepoo added this to the v0.1.0 milestone Oct 7, 2026
@Xuepoo Xuepoo added bug Something isn't working P1 Priority: high area:runtime Area: runtime orchestration area:lua Area: Lua runtime area:security Area: security corpus / audit labels Oct 7, 2026
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI (base), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dcff486e-5291-4c8d-9a1b-bf4e5ea034fe
📥 Commits

Reviewing files that changed from the base of the PR and between 6c3c261 and 0b440a2.

📒 Files selected for processing (4)
  • crates/bitty-lua/src/host.rs
  • crates/bitty-lua/tests/lua_parity.rs
  • crates/bitty-runtime/src/plugin_runtime/mod.rs
  • crates/bitty-runtime/src/plugin_runtime/services.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The host bridge now accepts environment keys that match ^[A-Z_][A-Z0-9_]*$ and are at most 64 bytes. Empty, malformed, and over-limit keys return E_ENV_KEY_INVALID before grant checks. Related documentation and tests reflect the updated contract.

Changes

Environment key contract

Layer / File(s) Summary
Define and enforce the key contract
crates/bitty-lua/src/host.rs, crates/bitty-runtime/src/plugin_runtime/mod.rs
The host bridge uses the uppercase-only key pattern and 64-byte limit. validate_env_key returns E_ENV_KEY_INVALID for empty, malformed, and over-limit keys. Runtime documentation reflects the pattern.
Check grant and bridge behavior
crates/bitty-lua/src/host.rs, crates/bitty-lua/tests/lua_parity.rs, crates/bitty-runtime/src/plugin_runtime/services.rs
Tests cover uppercase grant keys, lowercase and mixed-case rejection, lowercase authorization targets, validation before grant checks, and the updated Lua parity errors.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 0b440

The bridge intentionally rejects lowercase and over-64-byte keys under the updated contract, and the supplied implementation and test summaries agree. No actionable merge-blocking mismatch is evident; the change is mergeable subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0b440

The stricter validation preserves grant-gated environment access. Existing integrations may need updated keys or grants; compatibility with deployed configurations has not been confirmed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Within the inspected runtime path, a plugin generation can obtain values or presence information only for host-process environment keys covered by its exact or prefix grants. Prefix grants can cover multiple keys. The PR reduces admissible names rather than expanding this scope; deployment-specific secret contents and tenant or service exposure are not established.

Trust Boundaries and Controls

  • observed — Activation authority remains constrained by declared capabilities and recorded grants: undeclared recorded capabilities are rejected rather than silently accepted. Environment extraction then validates the resulting grant suffixes before VM creation. The inspected source provides no newly introduced route from arbitrary Lua keys to ambient environment reads.

Resilience and Maintainability Implications

  • observed — Grant replacement validates count and every entry before replacing the existing set. Newly invalid activation grants take the existing rollback path, which removes policy-host ownership, clears VM and service references, and releases transient capture and targeting ownership. The inspected failure path does not expose a partially authorized generation.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the change to tighten the bitty.env key bound and names the related Lua and runtime components.
Linked Issues check ✅ Passed [ #1751 ] The host bridge now uses a 64-byte limit and the ^[A-Z_][A-Z0-9_]*$ shape. Invalid keys return E_ENV_KEY_INVALID before grant checks. The updated unit, parity, and runtime tests cover in…
Out of Scope Changes check ✅ Passed All reported changes support [ #1751 ]: host validation and documentation, parity and runtime regression tests, and runtime grant-key documentation. The PR does not change ADR 0006, consistent with th…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@Xuepoo Xuepoo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Independent review (reviewer: pr-review-20261007-reviewer). Verdict: NEEDS-FIX — CI is red; code itself verifies clean.

Code verification (PR head 0b440a2):

  • ENV_KEY_MAX_BYTES 128 -> 64 (host.rs:193). Shape is uppercase-only: first byte is_ascii_uppercase|'' with all bytes uppercase|digit|'', length 1..=64, empty rejected — equivalent to ^[A-Z_][A-Z0-9_]*$. Length uses byte count, correct for the ASCII-only shape.
  • All validate_env_key failure modes (empty / over-bound / bad-first / bad-rest) now emit E_ENV_KEY_INVALID; zero env-key-related E_DEF_* remnants in the PR head. Trait docs and grant-extractor docs updated to the ADR bound.
  • env_grant_shape_ok delegates to env_key_shape_ok, so grants tighten consistently (lowercase can never grant); env_grant_authorizes gates keys on the same predicate. Matcher logic otherwise unchanged.
  • Tests pin the contract: accept HOME, _X1, A_B9, 64xA; reject "", 9LIVES, "has space", lower-ok?, home, Home, _x1, 65xA — all E_ENV_KEY_INVALID. Non-string arg stays E_DEF_INVALID (correct: argument-type error, not key shape). Grant tests pin lowercase rejection.
  • Scope is 4 files, all env-key paths (host.rs, lua_parity.rs, plugin_runtime/mod.rs docs, services.rs test). No grant-gating, audit, or redaction behavior change. Hygiene OK: Closes #1751, Task CTX-1004, labels/milestone set, branch ctx-1004/fix-env-key-adr0006.

Blocker (not caused by this diff, still merge-blocking):

  • Quality gates (2) FAIL: 2784/2785 pass, 1 failure in bitty-runtime::plugin_debug trace_handles_are_isolated_between_plugins (tests/plugin_debug.rs:314, xuepoo.tracer:start Lua error) — unrelated module, no env path. Other jobs (Linux/Windows/macOS, Quality gates 1) still pending; MSRV/Supply-chain/VM-tier pass. CodeRabbit review still in progress with no findings posted yet. mergeStateStatus BLOCKED, REVIEW_REQUIRED.

Action: re-run/investigate the plugin_debug failure, wait for full CI green plus CodeRabbit completion. Do not merge while red. I will re-review on a green head.

@Xuepoo
Xuepoo merged commit d5ca8e5 into main Oct 7, 2026
26 of 27 checks passed
@Xuepoo
Xuepoo deleted the ctx-1004/fix-env-key-adr0006 branch October 7, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:lua Area: Lua runtime area:runtime Area: runtime orchestration area:security Area: security corpus / audit bug Something isn't working P1 Priority: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tighten bitty.env key bound to ADR 0006 (64/uppercase/E_ENV_KEY_INVALID)

1 participant