Skip to content

fix(windows): soft-fail icacls ACL harden so Kimi auth is not blocked#157

Closed
Wibias wants to merge 1 commit into
lidge-jun:mainfrom
Wibias:fix/windows-acl-soft-fail
Closed

fix(windows): soft-fail icacls ACL harden so Kimi auth is not blocked#157
Wibias wants to merge 1 commit into
lidge-jun:mainfrom
Wibias:fix/windows-acl-soft-fail

Conversation

@Wibias

@Wibias Wibias commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Required NTFS ACL hardening via icacls was failing on Windows with ETIMEDOUT / EPERM, blocking auth.json writes during Kimi login and token refresh (surfaced as invalid_api_key).
  • Switch to Bun.spawnSync with windowsHide, retries, and a process lock.
  • Soft-fail ETIMEDOUT / EPERM / EACCES on required writes so auth continues with chmod-only hardening.

Example error this fixes:

ACL hardening failed (EPERM) — filesystem may not support per-user NTFS ACLs

Test plan

  • bun test tests/windows-secret-acl.test.ts
  • On Windows: Kimi login + a coding request that refreshes tokens no longer fails with ACL hardening errors
  • Confirm a warning is logged when ACL soft-fails, but the request proceeds

Replace execFileSync icacls with Bun.spawnSync (windowsHide, retries, lock). Soft-fail ETIMEDOUT/EPERM/EACCES on required writes so login and token refresh continue with chmod-only hardening.
lidge-jun added a commit that referenced this pull request Jul 19, 2026
…shared deadline

Absorbs the intent of community PR #157 (source head e19f38b) with a
maintainer redesign per Sol review:

- icacls runs through Bun.spawnSync (windowsHide) via an injectable runner
  seam; errors keep honest codes — ETIMEDOUT only for real timeouts, EICACLS
  (with exit code) otherwise. The source PR's blanket EPERM mapping made every
  failure soft-failable, defeating required:true on secret publication.
- Soft-fail on required paths is TIMEOUT-ONLY (the reported field symptom:
  Kimi auth blocked behind a hung icacls). Real EPERM/EACCES still throw.
- /remove:g failures are verified with locale-independent /findsid per broad
  SID before being accepted as harmless; a remaining ACE propagates the error.
- One shared 5s deadline per harden call replaces stacked 3x15s retries that
  could stall loadConfig ~135s; timed-out paths are cached for the process
  lifetime so auth is never re-stalled.
- Dropped the module-local boolean lock (serialized nothing across processes;
  sync calls already serialize in-process).
- Failure paths are activation-tested via runner/platform/clock seams:
  timeout soft-fail + warn, EPERM throw, /findsid verification both ways,
  shared-deadline cutoff + no-retry cache.

Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Thanks for tackling this — the field symptom (Kimi auth blocked behind a hung icacls) was real, and the fix landed on dev as a maintainer redesign that keeps your diagnosis and Bun.spawnSync/windowsHide direction:

  • a676f80b — maintainer-authored (Co-authored-by: @Wibias, source head e19f38b named). The redesign diverges from the PR where review found safety gaps:
    1. every non-timeout icacls failure was mapped to EPERM and soft-failed, which defeated required:true on secret publication — errors now keep honest codes (ETIMEDOUT only for real timeouts, EICACLS otherwise) and only genuine timeouts soft-fail;
    2. /remove:g failures were swallowed — now verified with locale-independent icacls /findsid per broad SID before being accepted;
    3. 3×15s retries could stall loadConfig ~135s — replaced with one shared 5s deadline per harden call plus a timed-out-path cache;
    4. the module-local lock serialized nothing across processes — dropped.
      Failure paths are activation-tested via injected runner/platform/clock seams.

Full suite green (3,087 tests). Closing in favor of the dev-landed commit; ships with the next release.

@lidge-jun lidge-jun closed this Jul 19, 2026
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.

2 participants