Skip to content

fix: search safe Windows dependency directories for modules - #26

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:windows-tinyconnectors-loader
Sep 25, 2026
Merged

senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:windows-tinyconnectors-loader

Conversation

@senamakel

@senamakel senamakel commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Search only the admitted module's directory and System32 when loading Windows DLL dependencies.
  • Include the Win32 loader error code in an artifact-load refusal so missing dependencies can be diagnosed without exposing filesystem paths.

Why

OpenHuman 0.64.0 production reports Sentry issue 36653 for tinyconnectors.dll on Windows, with other modules failing on the same machine. LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR alone excludes System32 from dependency resolution. The published DLL imports system and VC runtime libraries. LOAD_LIBRARY_SEARCH_SYSTEM32 adds that trusted location without searching application or registered user DLL directories.

Verification

  • cargo fmt --all -- --check
  • cargo test --locked -p tinybus --no-default-features --features modules,macros (327 passed; 10 ignored before the follow-up restriction)
  • cargo test --locked -p tinybus --no-default-features --features modules,macros module::loader::tests (2 passed after the follow-up restriction)
  • Windows real-loader CI is the runtime gate; this macOS host cannot execute a Windows DLL.

@tinysweeper

tinysweeper Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

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

State: Ready for maintainer review
Priority: medium
Reviewed head: 1d483636481b
Updated: 1790358075 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 1
Tests 0 Noted findings 0
Documentation 1 Resolved findings 9
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

  • medium · critique · Limit the Win32 error-code claim to artifact-load failures — The error code is captured only when `LoadLibraryExW` fails. A module can also be refused later when `GetProcAddress` cannot find a required ABI symbol, but that path still returns (crates/tinybus/src/module/loader\.rs:235)

Resolved this pass

  • Restrict dependency resolution to trusted DLL directories
  • Limit the Win32 error-code claim to artifact-load failures
  • Restrict dependency resolution to trusted DLL directories
  • Limit the Win32 error-code claim to artifact-load failures
  • Restrict dependency resolution to trusted DLL directories
  • Restrict dependency resolution to trusted DLL directories
  • Limit the Win32 error-code claim to artifact-load failures
  • Restrict dependency resolution to trusted DLL directories
  • Limit the Win32 error-code claim to artifact-load failures

Before merge

None.

How this fits together

flowchart LR
  n0["gate_descriptor"]:::impacted
  n1["load"]:::impacted
  n2["open"]:::impacted
  n3["symbol"]:::impacted
  n4["..._refuses_missing_and_nul_containing_paths"]:::impacted
  n5["Handle"]:::impacted
  n1 -->|calls| n0
  n1 -->|calls| n2
  n1 -->|calls| n3
  n2 -->|uses| n5
  n3 -->|uses| n5
  n4 -->|calls| n1
  n4 -->|tests| n1
  n4 -->|calls| n2
  n4 -->|tests| n2
  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: Reviewed 2 files; 1 finding. _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer answered 502 Bad Gateway), so this review saw part of what the engine holds._
  • Evidence: crates/tinybus/src/module/loader\.rs — Limit the Win32 error-code claim to artifact-load failures

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The Windows loader now restricts dependency resolution to the admitted module directory and System32, and the Win32 error-code message is limited to artifact-load failures. The change is safe to merge. 1 file was not security-reviewed: docs/modules/module/README.md (prose or tabular data). _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer answered 502 Bad Gateway), so this review saw part of what the engine holds._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Adds `LOAD_LIBRARY_SEARCH_SYSTEM32` to the Windows loader's dependency search path, keeping resolution limited to the module directory and a trusted system location, and includes the Win32 error code in the load-failure message. Both prior findings are resolved; the change is safe to merge. _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer answered 502 Bad Gateway), so this review saw part of what the engine holds._

commits

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

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: This pull request corrects the Windows module loader to search System32 alongside the module directory, preventing dependency resolution failures. It includes the Win32 error code in the artifact-load error message for diagnostics. Both prior findings are resolved; no new issues introduced. _The code index is behind this pull request (indexed at `67a3a8af37ef`), so retrieved context may be out of date._ _3 memory call(s) failed (model: cortex: v1/answer answered 502 Bad Gateway), so this review saw part of what the engine holds._

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.003492
  • Tokens: 142405 input · 8755 output · 17068 cached · 173 embedding
Head State Pass summary
f11959367d20 changes requested 2 active finding(s), 0 resolved finding(s) (at 1790357847)
1d483636481b ready for maintainer review 1 active finding(s), 9 resolved finding(s) (at 1790358075)

tinysweeper 0.1.0

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 55 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 6f221844-19c9-40d4-bf2b-a260a6511988

📥 Commits

Reviewing files that changed from the base of the PR and between 6ee8258 and 1d48363.

📒 Files selected for processing (2)
  • crates/tinybus/src/module/loader.rs
  • docs/modules/module/README.md

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

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.0035 · 132,739 in / 10,830 out · 8,911 cached (7%)  · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 165 embedded
critique:    $0.0021 · 78,933 in  / 3,866 out  · 4,054 cached (5%)  · gpt-5.6-luna, deepseek-v4-flash
security:    $0.0008 · 29,739 in  / 874 out    · 1,785 cached (6%)  · gpt-5.6-luna
tests:       $0.0003 · 15,119 in  / 980 out    · 2,048 cached (14%) · deepseek-v4-flash
description: $0.0001 · 6,765 in   / 1,120 out  · 1,024 cached (15%) · deepseek-v4-flash

Comment thread docs/modules/module/README.md Outdated
Comment thread crates/tinybus/src/module/loader.rs Outdated
@tinysweeper tinysweeper Bot added the priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition. label Sep 25, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The previously-blocking findings are resolved. Clearing the changes request.

             $0.0035 · 142,405 in / 8,755 out · 17,068 cached (12%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 173 embedded
critique:    $0.0022 · 87,184 in  / 3,238 out · 9,914 cached (11%)  · gpt-5.6-luna, deepseek-v4-flash
security:    $0.0007 · 30,465 in  / 444 out   · 3,570 cached (12%)  · gpt-5.6-luna
tests:       $0.0003 · 15,459 in  / 1,429 out · 2,048 cached (13%)  · deepseek-v4-flash
description: $0.0002 · 6,972 in   / 1,629 out · 1,536 cached (22%)  · deepseek-v4-flash

return Err(Error::module_refused(
path,
"dynamic loader rejected the artifact",
format!("dynamic loader rejected the artifact (Windows error {code})"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

priority medium critique confident

Limit the Win32 error-code claim to artifact-load failures

The error code is captured only when LoadLibraryExW fails. A module can also be refused later when GetProcAddress cannot find a required ABI symbol, but that path still returns a generic refusal without including the Win32 code. Any documentation or diagnostic guarantee covering all Windows loader refusals therefore remains inaccurate; limit the claim to artifact/dependency loading failures or include the code for every relevant refusal.

[RULE] inaccurate-documentation ·

@tinysweeper tinysweeper Bot added priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later. and removed priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition. labels Sep 25, 2026
@senamakel
senamakel merged commit 9c30e79 into tinyhumansai:main Sep 25, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant