Migrate to Rust + v0.1.0: TSF IME with romaji→katakana input - #1
Conversation
Replace the Zig/zigwin32 hand-rolled COM scaffolding with a Rust crate built on windows-rs 0.60 (#[implement]). Implements a minimal TSF TIP: IClassFactory + DLL exports, activation + key-event sink, composition via synchronous edit sessions, an underline display attribute, and COM/profile/category registration. Romanization -> Ainu katakana is powered by the ainconv crate. Preserves the existing CLSID and profile GUIDs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
build.ps1 / install.ps1 for local build + register (self-elevating, Program Files install). GitHub Actions: CI (build + test on Windows) and Release (on v* tag -> portable zip + Inno Setup installer artifacts, attached to the GitHub Release). Inno Setup script, MIT LICENSE (2024-2026), README with install/usage/build docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Normalize the raw IME buffer before ainconv conversion: ti->ci and chi->ci / ch->c, shi->si (onset-gated), fu->hu, macron/circumflex long vowels->acute, and full-Unicode lowercasing. tsu is deliberately NOT folded (coda-t + onset-s headwords like 'satsuwe' would be corrupted), and ti/shi are onset-gated to protect consonant-cluster seams. Pure std; 23 unit tests, all passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 50 minutes and 19 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughThe project's implementation language is migrated entirely from Zig to Rust. A new Rust cdylib crate ( ChangesainuKey: Zig → Rust TSF IME Rewrite
Sequence Diagram(s)sequenceDiagram
participant TSF as WindowsTSF
participant KeySink as ITfKeyEventSink
participant EditSession as run_sync
participant Composition as handle_action
participant Ainconv as ainconv
TSF->>KeySink: OnKeyDown wparam lparam context
KeySink->>KeySink: decode(wparam, lparam) -> Action
KeySink->>KeySink: would_eat(action, buffer) -> bool
alt eaten
KeySink->>EditSession: run_sync client_id context callback
EditSession->>Composition: handle_action(context, action)
alt Insert(ch)
Composition->>Ainconv: normalize(buffer) + convert_latn_to_kana
Ainconv-->>Composition: kana UTF-16 text
Composition->>Composition: update_preedit (set TF_ST_CORRECTION + underline)
Composition-->>TSF: preedit + selection update
else Commit
Composition-->>TSF: write final kana + end composition
else Backspace/Cancel
Composition-->>TSF: update/clear preedit + caret
end
else not eaten
KeySink-->>TSF: passthrough (return false)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cargo fmt across the crate; document the four unsafe COM/DLL entry points so clippy (missing_safety_doc) is clean. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/text_input_processor.rs (1)
129-129: 💤 Low valueRemove dead code.
let _ = TF_INVALID_COOKIE;is a no-op that evaluates and discards the constant. The comment mentions clarity, but this adds confusion rather than documentation value. Remove the line entirely or replace with a comment if explanation is needed.🧹 Suggested removal
- let _ = TF_INVALID_COOKIE; // referenced for clarity; cookies are valid here🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/text_input_processor.rs` at line 129, Remove the dead code statement `let _ = TF_INVALID_COOKIE;` from the code. This line is a no-op that only evaluates and discards the constant without performing any meaningful work. If additional explanation about why cookies are valid at this point is needed for documentation, replace the dead code statement with a proper comment that clarifies the intent, rather than keeping the unnecessary variable assignment.src/key_event_sink.rs (1)
115-131: 💤 Low valueMinor redundancy:
decodecalled twice per eaten keystroke.
would_eat(line 125) callsdecode, then if eaten, line 127 callsdecodeagain. Sincedecodeis pure, this is correct but redundant. Consider returning theActionfromwould_eator caching it. Given keystroke frequency, this is acceptable as-is.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/key_event_sink.rs` around lines 115 - 131, The `OnKeyDown` method calls `decode(wparam, lparam)` twice per keystroke when the key is eaten: once inside the `would_eat` call (which internally decodes to check if the keystroke should be consumed) and again on line 127 after confirming it was eaten. To eliminate this redundancy, refactor the `would_eat` method to return both the boolean eaten flag and the decoded Action, then update the `OnKeyDown` method to use the returned Action instead of calling `decode` a second time. This avoids the duplicate computation of the same pure function result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 15-26: Replace the mutable version references in the workflow with
full-length commit SHAs for all three actions (the actions/checkout@v4 action,
the dtolnay/rust-toolchain@stable action, and the Swatinem/rust-cache@v2 action)
to mitigate supply chain risks from tag manipulation. Additionally, add
`persist-credentials: false` to the checkout step to prevent subsequent steps
from accessing Git credentials. Apply these same hardening changes consistently
across all workflow files that use these actions.
In @.github/workflows/release.yml:
- Around line 12-13: This release workflow with write permissions uses unpinned
action references that are vulnerable to supply-chain attacks since version tags
can be reassigned. Replace all action references (including actions/checkout,
and any other actions used in the workflow) with their specific commit SHAs
instead of version tags like `@v4`, `@stable`, or `@v2`. Additionally, add
persist-credentials: false to the actions/checkout step to prevent the
GITHUB_TOKEN from remaining in the environment after checkout completes,
reducing the risk of credential misuse in the write-scoped workflow.
In `@install.ps1`:
- Around line 34-43: After invoking regsvr32.exe in both the uninstall block
(the & regsvr32.exe /u /s call) and the corresponding registration call
elsewhere in the script, check the $LASTEXITCODE variable immediately after each
invocation to determine if the operation succeeded. If $LASTEXITCODE is
non-zero, write an error message and stop the script execution using exit with
an appropriate error code, rather than continuing as if the operation succeeded.
In `@src/display_attribute.rs`:
- Around line 155-160: The Skip() method currently always returns S_OK
regardless of whether the requested count of elements could actually be skipped.
Per COM contracts, it should return S_FALSE when unable to skip the full
requested count. Modify the Skip() method to track how many elements are
actually available to skip (currently only 1 element can be skipped since the
method sets done to true immediately), compare this against the ulcount
parameter, and return S_FALSE when ulcount > 1 or the enumerator is already
exhausted. Only return S_OK when the full requested count was successfully
skipped.
In `@src/guids.rs`:
- Around line 3-4: The comment describing the GUID differences in the file
header is inaccurate. Update the comment to correctly state that the GUID
differences occur in the first group (Data1) with hex digits EB, EC, and ED (not
CB and CC in the third group), and clarify that three GUIDs are defined (not
two). Ensure the corrected comment accurately reflects the actual GUID values
defined in the file.
In `@src/lib.rs`:
- Around line 39-43: The LOCK_COUNT used by DllCanUnloadNow is only managed by
LockServer, but TextService instances created in CreateInstance do not increment
the lock count and have no Drop implementation to decrement it, allowing the DLL
to unload while TextService objects exist and causing crashes. Add a call to
lock_module() in TextService::new() to increment LOCK_COUNT when a TextService
instance is created, and implement a Drop trait for TextService that calls
unlock_module() to decrement the count when instances are destroyed. This
ensures that DllCanUnloadNow will not return true while live TextService objects
remain referenced by clients.
- Around line 75-83: The DllGetClassObject function dereferences the rclsid
pointer without null validation before the comparison with GUID_TEXT_SERVICE,
and passes the riid pointer to factory.query() without checking if it is null.
Since the function already validates ppv at the start, add similar null checks
for both rclsid and riid parameters before they are dereferenced or used. Insert
these checks early in the unsafe block, after the ppv validation but before the
rclsid comparison, to prevent crashes from malformed COM callers.
- Around line 46-48: The unlock_module() function can decrement LOCK_COUNT below
zero because it performs an unconditional fetch_sub without validation. Add a
guard condition in unlock_module() to prevent the count from going negative,
such as checking that LOCK_COUNT is greater than zero before decrementing.
Additionally, locate the DllCanUnloadNow function and change its unload
condition check from <= 0 to == 0 to ensure the DLL only unloads when the lock
count is exactly zero, not when it's negative due to imbalanced lock/unlock
calls.
In `@src/registry.rs`:
- Around line 165-173: The `unregister_server()` function currently treats all
RegDeleteTreeW errors as success when it should only accept missing key errors.
In the error handling block (where `err != ERROR_SUCCESS`), replace the
unconditional `return Ok(())` with a check that only returns Ok for
ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND; propagate all other errors.
Additionally, in the `unregister_all()` function, replace the error-discarding
patterns in the calls to the three unregister functions (around lines 290-293)
with the `?` operator to propagate failures instead of silently converting them
to success.
- Around line 267-273: The ComInit::new() method ignores the return value of
CoInitializeEx, but the Drop implementation unconditionally calls
CoUninitialize. Per Windows COM semantics, CoUninitialize must only be called
when CoInitializeEx succeeds. Change ComInit::new() to capture the HRESULT
return value from CoInitializeEx and return a Result type, then modify the
ComInit struct to track whether initialization succeeded (using a boolean flag
or Option). Update the Drop implementation to only call CoUninitialize if the
HRESULT indicated success. Update register_all() and unregister_all() to
propagate the Result error from ComInit::new() instead of ignoring it.
Additionally, fix unregister_server() to properly propagate RegDeleteTreeW
errors instead of treating them as success, and fix unregister_all() to
propagate unregister errors instead of discarding them.
---
Nitpick comments:
In `@src/key_event_sink.rs`:
- Around line 115-131: The `OnKeyDown` method calls `decode(wparam, lparam)`
twice per keystroke when the key is eaten: once inside the `would_eat` call
(which internally decodes to check if the keystroke should be consumed) and
again on line 127 after confirming it was eaten. To eliminate this redundancy,
refactor the `would_eat` method to return both the boolean eaten flag and the
decoded Action, then update the `OnKeyDown` method to use the returned Action
instead of calling `decode` a second time. This avoids the duplicate computation
of the same pure function result.
In `@src/text_input_processor.rs`:
- Line 129: Remove the dead code statement `let _ = TF_INVALID_COOKIE;` from the
code. This line is a no-op that only evaluates and discards the constant without
performing any meaningful work. If additional explanation about why cookies are
valid at this point is needed for documentation, replace the dead code statement
with a proper comment that clarifies the intent, rather than keeping the
unnecessary variable assignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6790e88f-ddbd-43ff-9e35-d0b368d11812
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
.github/workflows/ci.yml.github/workflows/release.yml.gitignore.gitmodulesCargo.tomlLICENSEREADME.mdainuKey.defassets/resources.rcbuild.ps1build.rsbuild.zigbuild.zig.zondebug.ps1install.ps1installer/ainuKey.isslib/zigwin32resources.rcsrc/composition.rssrc/consts.zigsrc/display_attribute.rssrc/dllroot.zigsrc/edit_session.rssrc/factory.rssrc/factory.zigsrc/guids.rssrc/key_event_sink.rssrc/lib.rssrc/registry.rssrc/registry.zigsrc/romaji.rssrc/service.zigsrc/text_input_processor.rssrc/text_service.rssrc/thread_mgr_event_sink.rssrc/windows/category.zigsrc/windows/debug.zigsrc/windows/guid.zigsrc/windows/profile.zigsrc/windows/registry.zigsrc/windows/types.zigtools/generate_time.zig
💤 Files with no reviewable changes (18)
- lib/zigwin32
- build.zig.zon
- debug.ps1
- tools/generate_time.zig
- build.zig
- src/windows/debug.zig
- src/windows/registry.zig
- src/windows/types.zig
- src/service.zig
- src/factory.zig
- src/windows/guid.zig
- assets/resources.rc
- src/windows/category.zig
- src/consts.zig
- .gitmodules
- src/dllroot.zig
- src/registry.zig
- src/windows/profile.zig
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust (stable + msvc target) | ||
| uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| targets: x86_64-pc-windows-msvc | ||
| components: clippy, rustfmt | ||
|
|
||
| - name: Cache cargo | ||
| uses: Swatinem/rust-cache@v2 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/ci.yml | head -30Repository: mkpoli/ainuKey
Length of output: 843
🏁 Script executed:
# Check if there are other workflow files to understand the broader context
git ls-files .github/workflows/Repository: mkpoli/ainuKey
Length of output: 113
🌐 Web query:
GitHub Actions workflow security best practices commit SHA pinning 2025 2026
💡 Result:
As of June 2026, pinning GitHub Actions to a full-length commit SHA is considered a critical security best practice for mitigating supply chain risks [1][2][3]. Because tags and branches are mutable references, they can be altered or deleted by an actor who gains access to the action's repository, potentially leading to the execution of malicious code in your workflows [1][4][5]. Key aspects of SHA pinning in 2026 include: Commit SHA Immutability: Pinning to a full-length commit SHA ensures that your workflow runs the exact code you expect, as the SHA uniquely identifies a specific state of the repository that cannot be altered [6][5]. Policy Enforcement: As of August 2025, GitHub allows administrators to enforce SHA pinning via organization or repository-level policies [7]. When enabled, these policies cause any workflow using a tag or branch reference to fail, mandating the use of full-length SHAs for all actions, including those in the dependency tree [7][6]. Tooling and Automation: Dependabot is the primary tool for managing these updates, as it understands SHA-pinned actions and will automatically open pull requests to update them when new versions are released [6][2]. Implementation: To pin an action, replace the tag or branch name with the full-length SHA from the action's repository: - uses: actions/checkout@f43a0e5 # v4 Best practices often include adding a comment (as shown above) to track the version tag the SHA corresponds to, improving maintainability while retaining security [8][9]. Organizations are strongly encouraged to enable these policies to harden their CI/CD pipelines against unauthorized dependency changes [7][3].
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://github.blog/security/supply-chain-security/securing-the-open-source-supply-chain-across-github/
- 3: https://devopsil.com/articles/2026-03-22-github-actions-security-hardening
- 4: https://github.com/github/docs/blob/962a1c8dccb8c0f66548b324e5b921b5e4fbc3d6/content/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions.md
- 5: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 6: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 7: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 8: https://codeql.github.com/codeql-query-help/actions/actions-unpinned-tag/
- 9: https://stackoverflow.com/questions/78903499/how-do-i-pin-an-action-to-a-specific-sha
🌐 Web query:
GitHub Actions checkout persist-credentials security recommendation
💡 Result:
It is a security best practice to set persist-credentials: false in the actions/checkout step of your GitHub Actions workflows unless you explicitly require the ability to perform authenticated Git operations (such as pushing commits) within that job [1][2]. By default, actions/checkout is set to persist-credentials: true [2]. When enabled, the action configures the local Git environment with the authentication token (or SSH key) used to clone the repository [3][4]. While this facilitates commands like git fetch or git push for subsequent steps, it creates a potential security risk: any subsequent step or third-party action running in the same job can read these credentials from the runner's Git configuration [2][5][6]. If a malicious or compromised action is executed, it could potentially misuse these persisted credentials [2]. Setting persist-credentials: false prevents the checkout action from writing these credentials to the Git configuration, thereby reducing the attack surface by ensuring that secrets are not unnecessarily exposed to other parts of your workflow [1][2]. If you need to perform authenticated Git operations but still wish to follow security hardening practices, consider the following: - Use persist-credentials: false to avoid global configuration of credentials [2]. - If you need to push changes, use the GITHUB_TOKEN or a dedicated secret directly within your steps rather than relying on persisted Git credentials [2]. - Ensure your GITHUB_TOKEN permissions are configured using the principle of least privilege, providing only the specific permissions necessary for the job [3][6].
Citations:
- 1: https://cheatsheetseries.owasp.org/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.html
- 2: https://actsense.dev/vulnerabilities/unsafe_checkout/
- 3: https://github.com/actions/checkout/?tab=readme-ov-file
- 4: https://github.com/actions/checkout/tree/refs/heads/main
- 5: https://github.com/suzuki-shunsuke/ghalint/blob/main/docs/policies/013.md
- 6: Remove
persist-credentialsor change the default tofalseactions/checkout#485
🏁 Script executed:
cat -n .github/workflows/release.yml | head -40Repository: mkpoli/ainuKey
Length of output: 1538
🏁 Script executed:
# Check for any CODEOWNERS or workflow policy files
git ls-files .github/ | grep -E "(CODEOWNERS|policies|rulesets)"Repository: mkpoli/ainuKey
Length of output: 40
🌐 Web query:
GitHub Actions security policy enforcement organizations 2026
💡 Result:
As of June 2026, GitHub Actions security policy enforcement is centered on a centralized governance model using rulesets and enterprise-wide policy configurations [1][2][3]. Key components of GitHub Actions security policy enforcement include: Centralized Governance and Rulesets GitHub has transitioned from distributed, per-workflow configuration to a centralized model that utilizes the ruleset framework [1]. This allows organizations to define security policies in a single location, making protections visible and enforceable across multiple repositories [1]. These protections include: - Workflow Execution Restrictions: Organizations can restrict specific workflow events (e.g., limiting workflow_dispatch to maintainers) and prohibit sensitive event triggers like pull_request_target on external contributions to protect secrets and permissions [1]. - Evaluate Mode: Organizations can implement new security policies in "evaluate mode," which allows them to monitor the impact of a policy—surfacing workflows that would have been blocked—before enforcing it, thereby reducing operational risk [1]. Allowed Actions and Dependency Hardening Security administrators can enforce strict controls over which actions and reusable workflows are permitted [2][3]. - Policy Options: Administrators can restrict usage to only local (enterprise-owned) actions, or allow a curated list of non-enterprise actions [2][3]. - Explicit Blocking: Policies now support explicit blocking of specific actions or versions by using a "!" prefix in the allowed list, which overrides any other allow rules [4]. - SHA Pinning: To mitigate risks from compromised dependencies, administrators can mandate that all actions be pinned to a full-length commit SHA [4][3]. This prevents the automatic execution of malicious code injected into new branches or tags [4][5]. Enterprise and Organization Hierarchy Security policies follow a hierarchical inheritance model [6][7]. Enterprise owners can enforce policies that apply across all organizations within the enterprise [2][3]. If an enterprise policy is enforced, organization owners cannot override it with more permissive settings [8][6][9]. Similarly, organization-level settings constrain individual repository configurations [8][10]. This infrastructure is supported by broader 2026 security roadmap initiatives, which include enhancing real-time observability, introducing richer approval/attestation gates, and consolidating fragmented controls into a unified governance surface [1].
Citations:
- 1: https://github.blog/news-insights/product-news/whats-coming-to-our-github-actions-2026-security-roadmap/
- 2: https://github.com/github/docs/blob/main/content/admin/enforcing-policies/enforcing-policies-for-your-enterprise/enforcing-policies-for-github-actions-in-your-enterprise.md
- 3: https://docs.github.com/enterprise-cloud@latest/admin/enforcing-policies/enforcing-policies-for-your-enterprise/enforcing-policies-for-github-actions-in-your-enterprise
- 4: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 5: https://docs.github.com/en/enterprise-server@3.21/actions/reference/security/secure-use
- 6: https://docs.github.com/en/enterprise-server@3.0/admin/github-actions/enforcing-github-actions-policies-for-your-enterprise
- 7: https://docs.github.com/en/enterprise-cloud@latest/admin/concepts/security-and-compliance/enterprise-policies
- 8: https://github.com/github/docs/blob/main/content/organizations/managing-organization-settings/disabling-or-limiting-github-actions-for-your-organization.md
- 9: https://docs.github.com/en/enterprise-server@3.8/admin/policies/enforcing-policies-for-your-enterprise/enforcing-policies-for-github-actions-in-your-enterprise
- 10: https://github.com/github/docs/blob/main/content/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository.md
Pin workflow actions to commit SHAs and disable checkout credential persistence.
Lines 16, 19, and 25 use mutable version refs (@v4, @stable, @v2), and the checkout step lacks persist-credentials: false. Pinning to full-length commit SHAs mitigates supply chain risks (tags can be altered by unauthorized actors), and persist-credentials: false prevents subsequent steps or third-party actions from accessing Git credentials. Both are hardening best practices as of 2026. Note: the same unpinned actions appear in release.yml (lines 20, 23, 28) where the risk is higher due to permissions: contents: write.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 15-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 19-19: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 25-25: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 15 - 26, Replace the mutable version
references in the workflow with full-length commit SHAs for all three actions
(the actions/checkout@v4 action, the dtolnay/rust-toolchain@stable action, and
the Swatinem/rust-cache@v2 action) to mitigate supply chain risks from tag
manipulation. Additionally, add `persist-credentials: false` to the checkout
step to prevent subsequent steps from accessing Git credentials. Apply these
same hardening changes consistently across all workflow files that use these
actions.
Source: Linters/SAST tools
| permissions: | ||
| contents: write |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the release workflow file
find . -name "release.yml" -type f | head -20Repository: mkpoli/ainuKey
Length of output: 90
🏁 Script executed:
# Let's examine the release.yml file to verify the claims
cat -n .github/workflows/release.ymlRepository: mkpoli/ainuKey
Length of output: 4019
Pin all actions to commit SHAs and disable credential persistence in this write-scoped release workflow.
This workflow has contents: write permissions (Line 13) and uses unpinned action references (Lines 20, 23, 28, 82, 89). Version tags like @v4, @stable, and @v2 can be reassigned to newer commits by maintainers, creating a supply-chain vulnerability. Additionally, actions/checkout@v4 (Line 20) does not disable credential persistence, leaving GITHUB_TOKEN in the environment for potential misuse.
Upgrade to commit SHAs for all actions and add persist-credentials: false to the checkout step.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 12 - 13, This release workflow
with write permissions uses unpinned action references that are vulnerable to
supply-chain attacks since version tags can be reassigned. Replace all action
references (including actions/checkout, and any other actions used in the
workflow) with their specific commit SHAs instead of version tags like `@v4`,
`@stable`, or `@v2`. Additionally, add persist-credentials: false to the
actions/checkout step to prevent the GITHUB_TOKEN from remaining in the
environment after checkout completes, reducing the risk of credential misuse in
the write-scoped workflow.
Source: Linters/SAST tools
| if ($Uninstall) { | ||
| if (Test-Path $dst) { | ||
| & regsvr32.exe /u /s $dst | ||
| Remove-Item $installDir -Recurse -Force -ErrorAction SilentlyContinue | ||
| Write-Host "ainuKey unregistered and removed from $installDir" | ||
| } else { | ||
| Write-Host "Nothing to uninstall ($dst not found)." | ||
| } | ||
| Read-Host "Press Enter to close" | ||
| return |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "install.ps1" -type fRepository: mkpoli/ainuKey
Length of output: 72
🏁 Script executed:
cat -n ./install.ps1Repository: mkpoli/ainuKey
Length of output: 3296
🌐 Web query:
PowerShell $ErrorActionPreference external executable regsvr32 exit code
💡 Result:
In PowerShell, the $ErrorActionPreference variable does not automatically affect or respond to the exit codes of external executables like regsvr32.exe by default [1][2][3]. External programs are independent processes, and historically, PowerShell does not treat their non-zero exit codes as errors that trigger the ErrorActionPreference mechanism [1][2]. To handle the exit code of regsvr32.exe properly, you should rely on the automatic $LASTEXITCODE variable, which stores the exit code of the most recently executed external program [1][3]. Standard practice for checking regsvr32 success: regsvr32.exe /s "path\to\your.dll" if ($LASTEXITCODE -ne 0) { Write-Error "regsvr32 failed with exit code $LASTEXITCODE" } Modern PowerShell (7.4+) Integration: If you are using PowerShell 7.4 or later, you can opt into integration between external command exit codes and PowerShell's error handling by setting the $PSNativeCommandUseErrorActionPreference preference variable to $true [1][4][2]. When this is enabled, a non-zero exit code from an external command will trigger a PowerShell error, which then respects the $ErrorActionPreference setting [4][2]. If you set $ErrorActionPreference to 'Stop' in this configuration, a non-zero exit code will cause the script to terminate [4][2]. Regsvr32 Exit Codes: The regsvr32 utility uses the following common exit codes for diagnostic purposes [5]: 0: Success 1: Error parsing command line 2: OleInitialize failed 3: LoadLibrary failed 4: GetProcAddress failed 5: Registration function failed [6][5] Note that these codes are for diagnostic use and are not guaranteed to be contractual across all versions of Windows [5]. Always rely on $LASTEXITCODE and verify the logic for your specific use case [2][7].
Citations:
- 1: https://stackoverflow.com/questions/67743279/how-to-get-exit-code-from-powershell-script
- 2: https://stackoverflow.com/questions/57468522/powershell-and-process-exit-codes
- 3: https://stackoverflow.com/questions/48864988/powershell-with-git-command-error-handling-automatically-abort-on-non-zero-exi
- 4: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_error_handling?view=powershell-7.6
- 5: https://devblogs.microsoft.com/oldnewthing/20180920-00/?p=99785
- 6: https://stackoverflow.com/questions/22094309/regsvr32-exit-codes-documentation
- 7: https://stackoverflow.com/questions/11450153/powershell-analogue-of-bashs-set-e
Check regsvr32 exit codes before proceeding.
Lines 36 and 60 invoke regsvr32.exe, but failures won't be caught by $ErrorActionPreference='Stop' since external executables' exit codes are not automatically treated as errors. The script will report success while registration/unregistration actually failed.
Suggested fix
if ($Uninstall) {
if (Test-Path $dst) {
& regsvr32.exe /u /s $dst
+ if ($LASTEXITCODE -ne 0) {
+ throw "Unregister failed for $dst (exit code: $LASTEXITCODE)"
+ }
Remove-Item $installDir -Recurse -Force -ErrorAction SilentlyContinue
Write-Host "ainuKey unregistered and removed from $installDir"
@@
Write-Host "Copied DLL -> $dst"
Write-Host "Registering (regsvr32 will show a success/failure dialog)..."
& regsvr32.exe $dst
+if ($LASTEXITCODE -ne 0) {
+ throw "Register failed for $dst (exit code: $LASTEXITCODE)"
+}🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'install.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install.ps1` around lines 34 - 43, After invoking regsvr32.exe in both the
uninstall block (the & regsvr32.exe /u /s call) and the corresponding
registration call elsewhere in the script, check the $LASTEXITCODE variable
immediately after each invocation to determine if the operation succeeded. If
$LASTEXITCODE is non-zero, write an error message and stop the script execution
using exit with an appropriate error code, rather than continuing as if the
operation succeeded.
| fn Skip(&self, ulcount: u32) -> windows::core::Result<()> { | ||
| if ulcount >= 1 { | ||
| self.done.set(true); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Skip() should return S_FALSE when unable to skip the requested count.
Per the COM IEnumXxx::Skip contract, S_FALSE should be returned when fewer elements are skipped than requested. Currently, if ulcount > 1 (only 1 item exists) or if the enumerator is already exhausted, the method returns S_OK despite not skipping the full count.
🔧 Suggested fix
fn Skip(&self, ulcount: u32) -> windows::core::Result<()> {
+ if ulcount == 0 {
+ return Ok(());
+ }
+ let can_skip = if self.done.get() { 0 } else { 1 };
if ulcount >= 1 {
self.done.set(true);
}
- Ok(())
+ if can_skip < ulcount {
+ Err(windows::Win32::Foundation::S_FALSE.into())
+ } else {
+ Ok(())
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn Skip(&self, ulcount: u32) -> windows::core::Result<()> { | |
| if ulcount >= 1 { | |
| self.done.set(true); | |
| } | |
| Ok(()) | |
| } | |
| fn Skip(&self, ulcount: u32) -> windows::core::Result<()> { | |
| if ulcount == 0 { | |
| return Ok(()); | |
| } | |
| let can_skip = if self.done.get() { 0 } else { 1 }; | |
| if ulcount >= 1 { | |
| self.done.set(true); | |
| } | |
| if can_skip < ulcount { | |
| Err(windows::Win32::Foundation::S_FALSE.into()) | |
| } else { | |
| Ok(()) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/display_attribute.rs` around lines 155 - 160, The Skip() method currently
always returns S_OK regardless of whether the requested count of elements could
actually be skipped. Per COM contracts, it should return S_FALSE when unable to
skip the full requested count. Modify the Skip() method to track how many
elements are actually available to skip (currently only 1 element can be skipped
since the method sets done to true immediately), compare this against the
ulcount parameter, and return S_FALSE when ulcount > 1 or the enumerator is
already exhausted. Only return S_OK when the full requested count was
successfully skipped.
| //! From IDENTITY (authoritative). The two service GUIDs differ only in the last | ||
| //! hex digit of the third group (`…CB` vs `…CC`). |
There was a problem hiding this comment.
Correct the GUID description comment.
The comment states the GUIDs "differ only in the last hex digit of the third group (…CB vs …CC)", but:
- The actual GUID values show differences in the first group (Data1), not the third:
5ECECCEB→5ECECCEC→5ECECCED - The hex digits are
…EB→…EC→…ED, not…CB→…CC - The comment mentions "two service GUIDs" but three GUIDs are defined
📝 Suggested correction
-//! From IDENTITY (authoritative). The two service GUIDs differ only in the last
-//! hex digit of the third group (`…CB` vs `…CC`).
+//! From IDENTITY (authoritative). The three GUIDs differ only in the last
+//! hex digit of the first group (Data1): `…EB` → `…EC` → `…ED`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! From IDENTITY (authoritative). The two service GUIDs differ only in the last | |
| //! hex digit of the third group (`…CB` vs `…CC`). | |
| //! From IDENTITY (authoritative). The three GUIDs differ only in the last | |
| //! hex digit of the first group (Data1): `…EB` → `…EC` → `…ED`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/guids.rs` around lines 3 - 4, The comment describing the GUID differences
in the file header is inaccurate. Update the comment to correctly state that the
GUID differences occur in the first group (Data1) with hex digits EB, EC, and ED
(not CB and CC in the third group), and clarify that three GUIDs are defined
(not two). Ensure the corrected comment accurately reflects the actual GUID
values defined in the file.
| /// Module lock count; drives `DllCanUnloadNow`. | ||
| static LOCK_COUNT: AtomicI32 = AtomicI32::new(0); | ||
|
|
||
| pub(crate) fn lock_module() { | ||
| LOCK_COUNT.fetch_add(1, Ordering::SeqCst); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where module lock state is mutated and whether TextService lifetime is wired to it.
rg -nP --type=rust -C3 '\block_module\s*\(|\bunlock_module\s*\('
rg -nP --type=rust -C4 'impl\s+Drop\s+for\s+TextService|impl\s+TextService|fn\s+new\s*\(' src/text_service.rs src/factory.rsRepository: mkpoli/ainuKey
Length of output: 1815
🏁 Script executed:
#!/bin/bash
# Find DllCanUnloadNow implementation
rg -nP --type=rust 'DllCanUnloadNow|fn dll_can_unload_now'
# Search for Drop implementation for TextService
rg -nP --type=rust 'impl Drop for TextService' -A 5
# Search for all calls to lock_module and unlock_module to see full usage pattern
rg -nP --type=rust -C 2 'lock_module|unlock_module' | grep -v '^--$'
# Check the complete structure of src/lib.rs around the mentioned lines
sed -n '35,100p' src/lib.rsRepository: mkpoli/ainuKey
Length of output: 3321
🏁 Script executed:
#!/bin/bash
# Get the complete DllCanUnloadNow function implementation
sed -n '101,120p' src/lib.rs
# More thorough search for Drop implementation in the entire codebase
rg -n 'impl.*Drop' --type=rust
# Search for TextService destruction or any reference counting
rg -n 'drop\|Drop\|RefCell' src/text_service.rs | head -20Repository: mkpoli/ainuKey
Length of output: 756
🏁 Script executed:
#!/bin/bash
# Find where TextService instances are created in the factory
rg -n 'TextService' src/factory.rs -B 2 -A 2
# Check the CreateInstance implementation in factory.rs
sed -n '30,55p' src/factory.rs
# Verify what happens in QueryInterface and other methods that might touch TextService
rg -n 'CreateInstance\|TextService' src/factory.rs -A 5Repository: mkpoli/ainuKey
Length of output: 1788
🏁 Script executed:
#!/bin/bash
# Check if TextService implements Drop or has any cleanup logic
cat src/text_service.rs | head -100
# Verify TextService structure and any ref-counting or AddRef/Release
rg -n 'impl.*TextService|AddRef|Release' src/text_service.rs -A 3
# Check if there are any other places where lock_module/unlock_module are called
rg -n 'lock_module\|unlock_module' src/ --type=rustRepository: mkpoli/ainuKey
Length of output: 2946
TextService creation/destruction do not contribute to module lock count; DllCanUnloadNow can unload while live objects exist.
DllCanUnloadNow checks only LOCK_COUNT, which is incremented/decremented exclusively by LockServer (external COM locking). However, TextService instances created in CreateInstance do not call lock_module(), and no Drop implementation exists to call unlock_module() on destruction. This means the DLL can be unloaded while TextService objects remain referenced by clients, leading to crashes.
Module lock must be wired to TextService lifetime: either call lock_module() in TextService::new() and implement Drop to call unlock_module(), or track live object count separately in DllCanUnloadNow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib.rs` around lines 39 - 43, The LOCK_COUNT used by DllCanUnloadNow is
only managed by LockServer, but TextService instances created in CreateInstance
do not increment the lock count and have no Drop implementation to decrement it,
allowing the DLL to unload while TextService objects exist and causing crashes.
Add a call to lock_module() in TextService::new() to increment LOCK_COUNT when a
TextService instance is created, and implement a Drop trait for TextService that
calls unlock_module() to decrement the count when instances are destroyed. This
ensures that DllCanUnloadNow will not return true while live TextService objects
remain referenced by clients.
| pub(crate) fn unlock_module() { | ||
| LOCK_COUNT.fetch_sub(1, Ordering::SeqCst); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the repository structure
git ls-files | head -20Repository: mkpoli/ainuKey
Length of output: 379
🏁 Script executed:
#!/bin/bash
# Read src/lib.rs to see the current implementation
wc -l src/lib.rsRepository: mkpoli/ainuKey
Length of output: 73
🏁 Script executed:
#!/bin/bash
# Read the relevant sections of src/lib.rs
cat -n src/lib.rs | sed -n '1,100p'Repository: mkpoli/ainuKey
Length of output: 3976
🏁 Script executed:
#!/bin/bash
# Continue reading src/lib.rs to see DllCanUnloadNow and other functions
cat -n src/lib.rs | sed -n '100,133p'Repository: mkpoli/ainuKey
Length of output: 1174
🏁 Script executed:
#!/bin/bash
# Search for all usages of lock_module and unlock_module
rg -n "lock_module|unlock_module" src/Repository: mkpoli/ainuKey
Length of output: 299
🏁 Script executed:
#!/bin/bash
# Look for the specific lines mentioned - "Also applies to: 92-94"
# Let's see what's on those lines
cat -n src/lib.rs | sed -n '88,95p'Repository: mkpoli/ainuKey
Length of output: 440
🏁 Script executed:
#!/bin/bash
# Look at the factory.rs to understand how lock/unlock are used
cat -n src/factory.rsRepository: mkpoli/ainuKey
Length of output: 2306
Prevent negative module-lock counts and tighten unload check.
unlock_module() can push LOCK_COUNT below zero. When this occurs, DllCanUnloadNow incorrectly returns S_OK because it checks <= 0 instead of == 0. This masks lock/unlock imbalance and can cause premature DLL unloading.
Suggested fix
pub(crate) fn unlock_module() {
- LOCK_COUNT.fetch_sub(1, Ordering::SeqCst);
+ let _ = LOCK_COUNT.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |v| {
+ if v > 0 { Some(v - 1) } else { Some(0) }
+ });
}
@@
pub unsafe extern "system" fn DllCanUnloadNow() -> HRESULT {
- if LOCK_COUNT.load(Ordering::SeqCst) <= 0 {
+ if LOCK_COUNT.load(Ordering::SeqCst) == 0 {
S_OK
} else {
S_FALSE
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib.rs` around lines 46 - 48, The unlock_module() function can decrement
LOCK_COUNT below zero because it performs an unconditional fetch_sub without
validation. Add a guard condition in unlock_module() to prevent the count from
going negative, such as checking that LOCK_COUNT is greater than zero before
decrementing. Additionally, locate the DllCanUnloadNow function and change its
unload condition check from <= 0 to == 0 to ensure the DLL only unloads when the
lock count is exactly zero, not when it's negative due to imbalanced lock/unlock
calls.
| if ppv.is_null() { | ||
| return E_POINTER; | ||
| } | ||
| unsafe { | ||
| *ppv = std::ptr::null_mut(); | ||
| if *rclsid != GUID_TEXT_SERVICE { | ||
| return CLASS_E_CLASSNOTAVAILABLE; | ||
| } | ||
| // Build the factory and QI it into the out-param. `query` performs the |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "src" -type d 2>/dev/null | head -5Repository: mkpoli/ainuKey
Length of output: 40
🏁 Script executed:
cat -n src/lib.rs | head -100Repository: mkpoli/ainuKey
Length of output: 3976
🏁 Script executed:
grep -n "fn query" src/factory.rs -A 10Repository: mkpoli/ainuKey
Length of output: 40
🏁 Script executed:
find . -name "factory.rs" -type fRepository: mkpoli/ainuKey
Length of output: 75
🏁 Script executed:
ls -la src/ | head -20Repository: mkpoli/ainuKey
Length of output: 956
🏁 Script executed:
cat -n src/factory.rsRepository: mkpoli/ainuKey
Length of output: 2306
Guard rclsid and riid before dereference in DllGetClassObject.
Line 85 dereferences rclsid without null checking, and line 92 passes riid to factory.query() without validation. A malformed COM caller can crash the DLL entrypoint. The defensive check on ppv at line 80 establishes that input validation is already considered necessary here.
Suggested fix
pub unsafe extern "system" fn DllGetClassObject(
rclsid: *const GUID,
riid: *const GUID,
ppv: *mut *mut c_void,
) -> HRESULT {
- if ppv.is_null() {
+ if ppv.is_null() || rclsid.is_null() || riid.is_null() {
return E_POINTER;
}
unsafe {
*ppv = std::ptr::null_mut();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ppv.is_null() { | |
| return E_POINTER; | |
| } | |
| unsafe { | |
| *ppv = std::ptr::null_mut(); | |
| if *rclsid != GUID_TEXT_SERVICE { | |
| return CLASS_E_CLASSNOTAVAILABLE; | |
| } | |
| // Build the factory and QI it into the out-param. `query` performs the | |
| if ppv.is_null() || rclsid.is_null() || riid.is_null() { | |
| return E_POINTER; | |
| } | |
| unsafe { | |
| *ppv = std::ptr::null_mut(); | |
| if *rclsid != GUID_TEXT_SERVICE { | |
| return CLASS_E_CLASSNOTAVAILABLE; | |
| } | |
| // Build the factory and QI it into the out-param. `query` performs the |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib.rs` around lines 75 - 83, The DllGetClassObject function dereferences
the rclsid pointer without null validation before the comparison with
GUID_TEXT_SERVICE, and passes the riid pointer to factory.query() without
checking if it is null. Since the function already validates ppv at the start,
add similar null checks for both rclsid and riid parameters before they are
dereferenced or used. Insert these checks early in the unsafe block, after the
ppv validation but before the rclsid comparison, to prevent crashes from
malformed COM callers.
| fn unregister_server() -> windows::core::Result<()> { | ||
| let clsid_str = guid_braced(&GUID_TEXT_SERVICE); | ||
| let clsid_key = wide(&format!("CLSID\\{}", clsid_str)); | ||
| // SAFETY: clsid_key is a valid NUL-terminated wide string. | ||
| let err = unsafe { RegDeleteTreeW(HKEY_CLASSES_ROOT, PCWSTR(clsid_key.as_ptr())) }; | ||
| if err != ERROR_SUCCESS { | ||
| // Best-effort: a missing key is fine. | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -n "registry.rs" -t fRepository: mkpoli/ainuKey
Length of output: 289
🏁 Script executed:
head -n 350 src/registry.rs | tail -n +160Repository: mkpoli/ainuKey
Length of output: 4061
🏁 Script executed:
sed -n '150,180p' src/registry.rsRepository: mkpoli/ainuKey
Length of output: 1001
Propagate real unregistration failures instead of silently treating all errors as success.
The unregister_server() function treats all RegDeleteTreeW errors as success (lines 157-160), despite the comment suggesting only missing keys are acceptable. Additionally, unregister_all() discards errors from all three unregister functions (line 290-293), allowing stale COM/TSF registration to persist while reporting success. This can leave the system in an inconsistent state.
Suggested fix: Accept only ERROR_FILE_NOT_FOUND and ERROR_PATH_NOT_FOUND as expected conditions; propagate other failures as errors. Replace error-discarding patterns in unregister_all() with ? operator to propagate failures:
Suggested fix
-use windows::Win32::Foundation::{ERROR_SUCCESS, E_FAIL, MAX_PATH};
+use windows::Win32::Foundation::{
+ ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, ERROR_SUCCESS, E_FAIL, MAX_PATH
+};
@@
fn unregister_server() -> windows::core::Result<()> {
let err = unsafe { RegDeleteTreeW(HKEY_CLASSES_ROOT, PCWSTR(clsid_key.as_ptr())) };
- if err != ERROR_SUCCESS {
- // Best-effort: a missing key is fine.
- return Ok(());
- }
- Ok(())
+ if err == ERROR_SUCCESS || err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND {
+ Ok(())
+ } else {
+ Err(E_FAIL.into())
+ }
}
@@
pub fn unregister_all() -> windows::core::Result<()> {
let _com = ComInit::new();
- let _ = unregister_profile();
- let _ = unregister_categories();
- let _ = unregister_server();
+ unregister_profile()?;
+ unregister_categories()?;
+ unregister_server()?;
Ok(())
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/registry.rs` around lines 165 - 173, The `unregister_server()` function
currently treats all RegDeleteTreeW errors as success when it should only accept
missing key errors. In the error handling block (where `err != ERROR_SUCCESS`),
replace the unconditional `return Ok(())` with a check that only returns Ok for
ERROR_FILE_NOT_FOUND or ERROR_PATH_NOT_FOUND; propagate all other errors.
Additionally, in the `unregister_all()` function, replace the error-discarding
patterns in the calls to the three unregister functions (around lines 290-293)
with the `?` operator to propagate failures instead of silently converting them
to success.
| fn new() -> Self { | ||
| // SAFETY: initializing an STA on the registration thread. | ||
| unsafe { | ||
| let _ = CoInitializeEx(None, COINIT_APARTMENTTHREADED); | ||
| } | ||
| ComInit | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/registry.rs | sed -n '260,300p'Repository: mkpoli/ainuKey
Length of output: 834
🏁 Script executed:
cat -n src/registry.rs | sed -n '165,180p'Repository: mkpoli/ainuKey
Length of output: 660
🏁 Script executed:
cat -n src/registry.rs | sed -n '240,275p'Repository: mkpoli/ainuKey
Length of output: 1193
🌐 Web query:
CoInitializeEx CoUninitialize Windows COM error handling unbalanced initialization
💡 Result:
In Windows COM development, balancing calls to CoInitializeEx and CoUninitialize is critical because COM maintains a reference count for the library on a per-thread basis [1][2][3]. Every successful call to CoInitialize or CoInitializeEx—including those that return S_OK or S_FALSE—must be matched by exactly one call to CoUninitialize [1][2]. If your initialization logic is unbalanced, the following consequences and handling strategies apply: Core Rules for Balancing 1. Match Every Success: You must only call CoUninitialize for calls to CoInitializeEx that succeeded (returned S_OK or S_FALSE) [3]. 2. Handle Failure Codes: If CoInitializeEx fails (e.g., returns RPC_E_CHANGED_MODE), you must NOT call CoUninitialize for that specific failed attempt [3]. Calling it anyway can prematurely shut down the COM library on that thread, potentially causing crashes or errors for other code that expects COM to remain active [3]. 3. Thread Specificity: COM initialization is per-thread [1]. You must initialize and uninitialize COM on the same thread [1][2]. 4. DllMain Constraint: Never call CoInitialize, CoInitializeEx, or CoUninitialize from DllMain, as the loader lock prevents safe control over the order of DLL loading and unloading [1][2]. Error Handling and Best Practices - Checking Return Values: Always check the HRESULT returned by CoInitializeEx [4][3]. If you receive RPC_E_CHANGED_MODE, it indicates that COM is already initialized on the thread but with a different, incompatible concurrency model [1][5]. In this case, do not proceed with your own uninitialization logic for that failed call [3]. - RAII Patterns: To ensure robust balancing, especially in code that uses exceptions, use the RAII (Resource Acquisition Is Initialization) pattern [3]. Create a wrapper class that calls CoInitializeEx in its constructor and CoUninitialize in its destructor, ensuring the destructor only calls CoUninitialize if the constructor succeeded [3]. - OleInitialize Note: If your application uses OleInitialize, it handles COM initialization internally [2][5]. Do not call CoUninitialize separately if you are using OleUninitialize, as the latter calls CoUninitialize automatically [2].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-coinitializeex
- 2: https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-couninitialize
- 3: https://learn.microsoft.com/en-us/archive/blogs/benkuhn/handling-coinitialize-and-couninitialize
- 4: https://learn.microsoft.com/en-us/windows/win32/learnwin32/initializing-the-com-library
- 5: https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-coinitializeex?redirectedfrom=MSDN
🌐 Web query:
windows CoInitializeEx possible errors HRESULT failure modes
💡 Result:
When calling the CoInitializeEx function in Windows, HRESULT values indicate the success or failure of the COM library initialization on the current thread [1][2]. Success and Standard Codes - S_OK: The COM library was initialized successfully on the current thread [1][3]. - S_FALSE: The COM library is already initialized on this thread [1][3]. Multiple successful calls are permitted provided the concurrency model (the dwCoInit parameter) remains consistent; subsequent calls return S_FALSE and must still be balanced by a corresponding call to CoUninitialize [1][4]. - E_INVALIDARG, E_OUTOFMEMORY, E_UNEXPECTED: Standard COM failure codes that may be returned under various error conditions [1][3]. Failure Modes and Specific Errors - RPC_E_CHANGED_MODE (0x80010106): This is the most common failure mode for CoInitializeEx [5]. It occurs when the thread has already been initialized with a different concurrency model (e.g., attempting to initialize as MTA after it has already been initialized as STA, or vice versa) [1][3]. This can also happen if a third-party API or helper function (such as HtmlHelp) implicitly initializes the thread in a specific model before your code explicitly calls CoInitializeEx [6]. - CO_E_NOTINITIALIZED (0x800401F0): While this is not returned by CoInitializeEx itself, it is the primary error code returned by subsequent COM functions if the thread fails to call CoInitializeEx (or CoInitialize) beforehand [1][3]. - Implicit MTA Issues: If another thread in the process has initialized COM using COINIT_MULTITHREADED, uninitialized threads may implicitly inherit that Multi-Threaded Apartment (MTA) status [7]. Relying on this implicit state is dangerous because the MTA could be torn down if the thread that originally initialized it calls CoUninitialize, leaving the implicitly initialized thread in an invalid state [7]. General Best Practices - Every successful call to CoInitializeEx—including those that return S_FALSE—must be balanced by a corresponding call to CoUninitialize [1][2]. - Libraries and DLLs should generally avoid calling CoInitializeEx on behalf of an application, as they cannot safely determine the application's required threading model, leading to potential RPC_E_CHANGED_MODE failures [8].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-coinitializeex
- 2: https://learn.microsoft.com/en-us/windows/win32/learnwin32/initializing-the-com-library
- 3: https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-coinitializeex?redirectedfrom=MSDN
- 4: https://techshelps.github.io/MSDN/COM/devdoc/live/com/api1_5iyg.htm
- 5: https://learn.microsoft.com/en-us/answers/questions/1191967/api-method-coinitializeex-always-return-s-false-or
- 6: https://learn.microsoft.com/en-us/troubleshoot/windows/win32/coinitializeex-fails-after-calling-htmlhelp-same-thread
- 7: https://devblogs.microsoft.com/oldnewthing/20130419-00/?p=4613
- 8: https://learn.microsoft.com/en-us/archive/blogs/larryosterman/things-you-shouldnt-do-part-2-dlls-cant-ever-call-coinitialize-on-the-applications-thread
🏁 Script executed:
cat -n src/registry.rs | grep -n "fn unregister_server\|fn unregister_profile\|fn unregister_categories"Repository: mkpoli/ainuKey
Length of output: 258
🏁 Script executed:
cat -n src/registry.rs | sed -n '195,230p'Repository: mkpoli/ainuKey
Length of output: 1683
🏁 Script executed:
cat -n src/registry.rs | sed -n '153,175p'Repository: mkpoli/ainuKey
Length of output: 1059
🏁 Script executed:
cat -n src/registry.rs | sed -n '281,287p'Repository: mkpoli/ainuKey
Length of output: 310
Check CoInitializeEx result and only call CoUninitialize on success.
ComInit::new() ignores the CoInitializeEx return value (line 258) but Drop unconditionally calls CoUninitialize() (line 268). Per Windows COM semantics, CoUninitialize() must only be called when CoInitializeEx succeeds; calling it after a failure (e.g., RPC_E_CHANGED_MODE, E_OUTOFMEMORY) corrupts the COM library state and causes crashes.
Additionally:
register_all()(line 274) andunregister_all()(line 282) must propagateComInit::new()errors once it returnsResult.unregister_server()(lines 157–160) treats allRegDeleteTreeWerrors as success.unregister_all()(lines 283–285) discards all unregister errors, masking real failures.
Suggested fix
-struct ComInit;
+struct ComInit {
+ initialized: bool,
+}
@@
impl ComInit {
- fn new() -> Self {
+ fn new() -> windows::core::Result<Self> {
unsafe {
- let _ = CoInitializeEx(None, COINIT_APARTMENTTHREADED);
+ CoInitializeEx(None, COINIT_APARTMENTTHREADED)?;
}
- ComInit
+ Ok(ComInit { initialized: true })
}
}
@@
impl Drop for ComInit {
fn drop(&mut self) {
- unsafe {
- CoUninitialize();
- }
+ if self.initialized {
+ unsafe { CoUninitialize(); }
+ }
}
}
@@
pub fn register_all() -> windows::core::Result<()> {
- let _com = ComInit::new();
+ let _com = ComInit::new()?;
@@
pub fn unregister_all() -> windows::core::Result<()> {
- let _com = ComInit::new();
+ let _com = ComInit::new()?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/registry.rs` around lines 267 - 273, The ComInit::new() method ignores
the return value of CoInitializeEx, but the Drop implementation unconditionally
calls CoUninitialize. Per Windows COM semantics, CoUninitialize must only be
called when CoInitializeEx succeeds. Change ComInit::new() to capture the
HRESULT return value from CoInitializeEx and return a Result type, then modify
the ComInit struct to track whether initialization succeeded (using a boolean
flag or Option). Update the Drop implementation to only call CoUninitialize if
the HRESULT indicated success. Update register_all() and unregister_all() to
propagate the Result error from ComInit::new() instead of ignoring it.
Additionally, fix unregister_server() to properly propagate RegDeleteTreeW
errors instead of treating them as success, and fix unregister_all() to
propagate unregister errors instead of discarding them.
Per-branch, per-feature manual test steps with expected behavior and the most-likely first failure modes, so the Windows-in-the-loop validation is mechanical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an Ubuntu job that builds ainukey.dll with cargo-xwin (lld-link + MSVC libs) and asserts DllGetClassObject/DllCanUnloadNow/DllRegisterServer/DllUnregisterServer are exported. Verified locally: all branches produce a valid PE32+ DLL exporting the four entry points (so regsvr32 can register it) — link/export coverage beyond cargo check, on cheap runners. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate to Rust + v0.1.0: TSF IME with romaji→katakana input
Summary
Complete rewrite of ainuKey from Zig to Rust as a Windows TSF text input processor, plus the first functional release.
Migration (Zig → Rust)
#[implement]).ainconvcrate. Existing CLSID/profile GUIDs preserved.Robust romaji input (
ti→ci)src/romaji.rsnormalizes the buffer before conversion:ti→ci,chi→ci/ch→c,shi→si(onset-gated),fu→hu, macron/circumflex long vowels → acute, full-Unicode lowercasing.tsu→tudeliberately not applied (would corrupt real headwords likesatsuwe);ti/shionset-gated to protect cluster seams. 23 unit tests, all passing.Packaging / release
v*tag → portable zip and Inno Setup installer, attached to the Release).build.ps1/install.ps1(self-elevating, Program Files install), MIT LICENSE (2024–2026), updated README.Verification
cargo check --target x86_64-pc-windows-msvcgreen on rustc 1.96.0.Notes
ja-JPlangid (fallback). Registering as the Ainu language (transient LCID) is tracked for v0.2.Summary by CodeRabbit
TESTING.md..gitignoreto exclude generated build/release outputs.