fix(module): check Windows DLL owner and ACL before load

[senamakel]

Summary

• Check each Windows module DLL's owner and ACL before loading it.
• Require directory and file owners to be the running account, Administrators, or SYSTEM.
• Keep the inherited CREATOR OWNER directory allowance from fix(module): accept Windows creator-owner module cache ACL #25 while refusing a DLL writable by Everyone.

Why

PR #25 fixed OpenHuman's Windows module-cache refusal and was merged while its security review finished. The review correctly noted that an inherited CREATOR OWNER ACE on a directory can grant rights to a child object's owner. A hostile preexisting DLL owner could then write the file even when the parent directory itself is private. This follow-up checks the actual module file before load.

Validation

• cargo fmt --all -- --check
• cargo test --locked -p tinybus --features modules module::host::tests --lib (33 passed, 10 fixture-dependent ignored on macOS)
• Windows CI exercises current-user cache admission and rejects Everyone write grants on directories and files.

Related

• Follow-up to fix(module): accept Windows creator-owner module cache ACL #25 and its ACL review threads.
• OpenHuman Sentry issue: https://sentry.tinyhumans.ai/organizations/tinyhumans/issues/36648/

Summary by CodeRabbit

• Bug Fixes
  • Improved Windows module security checks by rejecting artifact files and directories that grant write access to untrusted parties.
  • Windows paths are now accepted only when their ownership and write permissions meet the trusted access requirements. This helps prevent modules from being loaded from locations that could be modified by unauthorized users.

[tinysweeper]

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Changes requested
Priority: high
Reviewed head: 1ab607c67264
Updated: 1790358937 (Unix time)

Review snapshot

Change surface	Files	Review signal	Count
Production	1	Active findings	1
Tests	1	Noted findings	0
Documentation	0	Resolved findings	0
Configuration	0	Pending checks/questions	0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

• high · description · Redact the path from the module-refused error — The new error passes the raw `path` to `Error::module_refused`, which constructs an error containing that value. The repository rule states: "Errors must never carry the value that (\(pull request description\))

Before merge

• Address Redact the path from the module-refused error (\(pull request description\)).

How this fits together

flowchart LR
  n0["check_directory<br/>changed"]:::changed
  n1["check_file<br/>changed"]:::changed
  n2["windows_directory_grants_untrusted_write<br/>changed"]:::changed
  n3["ModuleInfo"]:::impacted
  n4["load_dir"]:::impacted
  n5["new"]:::impacted
  n6["register_lazy"]:::impacted
  n7["scan_dir"]:::impacted
  n8["Err"]:::impacted
  n0 -->|calls| n2
  n0 -->|calls| n8
  n1 -->|calls| n5
  n1 -->|calls| n8
  n2 -->|calls| n8
  n4 -->|calls| n0
  n4 -->|calls| n1
  n4 -->|uses| n3
  n4 -->|calls| n5
  n4 -->|calls| n6
  n4 -->|calls| n8
  n6 -->|calls| n1
  n6 -->|uses| n3
  n6 -->|calls| n5
  n6 -->|calls| n8
  n7 -->|calls| n0
  n7 -->|calls| n1
  n7 -->|uses| n3
  n7 -->|calls| n5
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff

Loading

Agent review details

critique

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: The change adds Windows ACL checks for both module files and directories, including owner validation for CREATOR OWNER handling. It looks safe to merge based on the provided implementation and tests. _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

security

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: The Windows module admission change strengthens ownership checks for both directories and individual artifacts, and the added coverage exercises the new file gate. No security issue is introduced by this diff. _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

tests

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: The change extends Windows directory write-ownership checks to individual module files, adds owner validation before ACE traversal, and includes two new integration tests covering the acceptance of a user-owned directory and refusal of a world-writable module file. The tests assert specific outcomes and would fail on regression. _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

commits

• Conclusion: Neutral
• Scope reviewed: all assigned evidence
• Lane summary: Nothing sensitive found in what this pull request commits.

description

• Conclusion: Failure
• Scope reviewed: all assigned evidence
• Lane summary: Adds owner and ACL checks before loading Windows module DLLs. The change is logically correct but introduces an error that includes the file path, violating the rule against carrying values in errors. _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
• Evidence: \(pull request description\) — Redact the path from the module-refused error

e2e

• Conclusion: Neutral
• Scope reviewed: all assigned evidence
• Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.

Evidence and run details

• Models: ladder/vectors, gpt-5.6-luna, deepseek-v4-flash
• Spend: $0.002747
• Tokens: 104754 input · 12103 output · 13412 cached · 381 embedding

Head	State	Pass summary
1ab607c67264	changes requested	1 active finding(s), 0 resolved finding(s) (at 1790358937)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 428f87d5-ccb7-40d3-b58b-86db44adc5fc

📥 Commits

Reviewing files that changed from the base of the PR and between 50eae54 and 1ab607c.

📒 Files selected for processing (2)

• crates/tinybus/src/module/host.rs
• crates/tinybus/src/module/host_test.rs

 ____________________________________________
< Like a moth to a flame, I'm drawn to bugs. >
 --------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

---

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

[tinysweeper]

Requesting changes: 1 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0027 · 104,754 in / 12,103 out · 13,412 cached (13%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 381 embedded
critique:    $0.0008 · 32,200 in  / 649 out    · 4,224 cached (13%)  · gpt-5.6-luna
security:    $0.0013 · 51,239 in  / 1,510 out  · 5,604 cached (11%)  · gpt-5.6-luna
tests:       $0.0003 · 13,298 in  / 2,281 out  · 2,048 cached (15%)  · deepseek-v4-flash
description: $0.0002 · 4,786 in   / 3,836 out  · 1,536 cached (32%)  · deepseek-v4-flash