Skip to content

fix(security): surface Windows ACL repair hint when .secret_key is unreadable#1748

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
Lunar-feedmob:fix/windows-secret-key-permission-error
May 15, 2026
Merged

fix(security): surface Windows ACL repair hint when .secret_key is unreadable#1748
senamakel merged 2 commits into
tinyhumansai:mainfrom
Lunar-feedmob:fix/windows-secret-key-permission-error

Conversation

@Lunar-feedmob
Copy link
Copy Markdown
Contributor

@Lunar-feedmob Lunar-feedmob commented May 14, 2026

Summary

When .secret_key exists but cannot be read on Windows (ACL/permission issue from the installer), the error now includes:

  1. The full file path (so users know which file is blocked)
  2. Actionable icacls commands to repair permissions

Previously the error was a generic "Failed to read secret key file" that users mistook for an OAuth failure (see #1742 — both Google and GitHub sign-in appeared broken).

Problem

On Windows 11 with the official installer, .openhuman/.secret_key can be created with incorrect ACL/ownership, making it unreadable. The app shows a generic OAuth failure instead of pointing to the real issue.

Changes

  • src/openhuman/security/secrets.rs: Replace .context("Failed to read secret key file") with .with_context(|| ...) that:
    • Always includes the file path
    • On Windows (#[cfg(windows)]), appends specific icacls /reset repair commands

Example error output (Windows)

Failed to read secret key file at C:\Users\alice\.openhuman\.secret_key

This is often caused by incorrect file permissions on Windows. Try repairing ACLs on the .openhuman directory:
icacls "%USERPROFILE%\.openhuman" /reset /t /c
icacls "%USERPROFILE%\.openhuman\.secret_key" /reset /c

Testing

  • #[cfg(windows)] guard ensures zero impact on macOS/Linux builds
  • The existing retry logic in read_key_file_with_retry is unchanged
  • Non-Windows platforms see only the improved path-inclusive message

Closes #1742

Summary by CodeRabbit

  • Bug Fixes
    • Improved error messages for secret key loading failures: messages now include the key file path and actionable remediation guidance to help resolve permission or access issues. On Windows, guidance includes specific commands to fix directory/file permissions for quicker recovery.

Review Change Stack

…key is unreadable

When the secret key file exists but cannot be read on Windows, the error
now includes the file path and actionable icacls commands to repair
permissions. Previously the error was a generic 'Failed to read secret
key file' that looked like an OAuth failure.

Closes tinyhumansai#1742
@Lunar-feedmob Lunar-feedmob requested a review from a team May 14, 2026 15:09
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 23f42ee8-8ea3-4955-89f2-e950c32a7162

📥 Commits

Reviewing files that changed from the base of the PR and between bbd01e0 and 6e99669.

📒 Files selected for processing (1)
  • src/openhuman/security/secrets.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/openhuman/security/secrets.rs

📝 Walkthrough

Walkthrough

SecretStore::load_or_create_key now enriches key-file read failures with the failing path and, on Windows, adds actionable icacls remediation commands for repairing directory and key file ACL permissions.

Changes

Windows Secret Key Error Recovery

Layer / File(s) Summary
Enhanced error context for key-file read failures
src/openhuman/security/secrets.rs
load_or_create_key wraps read_key_file_with_retry failures with the key file path and, on Windows, appends icacls commands to repair .openhuman directory and .secret_key ACLs for permission troubleshooting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • tinyhumansai/openhuman#1509: Introduces the read_key_file_with_retry behavior that this PR enhances with richer error context and Windows ACL repair guidance.

Poem

🐰 A humble rabbit's key stood locked,
Windows ACLs blocked the way,
But now the error speaks so clear:
"Try icacls, friend, don't dismay!"
Permission troubles melt away.

🚥 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 summarizes the main change: adding Windows ACL repair guidance when the secret key file is unreadable, which directly addresses the linked issue.
Linked Issues check ✅ Passed The PR partially addresses issue #1742 by surfacing better error context on Windows with icacls commands, but does not fully implement the suggested improvements like a dedicated recovery action or repair script.
Out of Scope Changes check ✅ Passed All changes are within scope: the modification to error reporting in SecretStore::load_or_create_key directly addresses the Windows ACL issue described in #1742.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@src/openhuman/security/secrets.rs`:
- Around line 191-203: The Rust code block building the error message for
read_key_file_with_retry(&self.key_path) (assigning to hex_key) is misformatted
and failing CI; run rustfmt (cargo fmt --all) to reformat the file, then commit
the formatted changes so the message construction around self.key_path and the
Windows-specific cfg block matches project style and passes cargo fmt/prettier
checks.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74938fb1-36ae-4465-a528-05f2deebd369

📥 Commits

Reviewing files that changed from the base of the PR and between 4d5e0d5 and bbd01e0.

📒 Files selected for processing (1)
  • src/openhuman/security/secrets.rs

Comment thread src/openhuman/security/secrets.rs
Break long format!() line to satisfy cargo fmt check.
@senamakel senamakel merged commit c8b8aeb into tinyhumansai:main May 15, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows sign-in fails due to unreadable local .secret_key

2 participants