fix(lark): enforce private ACLs on Windows credential tree (#5141)

Browse Source

* fix(lark): enforce private ACLs on Windows credential tree

On Windows, posix chmod(0o700/0o600) does not map to NTFS ACLs, so the
secret-bearing Lark CLI credential tree was not actually owner-restricted
and existing trees were not repaired.

Branch the permission application by platform:
- POSIX: directories 0o700, files 0o600 (behavior unchanged).
- Windows: disable inherited ACLs, grant the Gateway process user Full
  Control (resolved via its SID from whoami /user /fo csv /nh so it is
  locale-independent), and remove broad non-administrative grants
  (Everyone, Authenticated Users, Users). Fail closed on identity or
  icacls failures so a tree is never left accessible silently.

Existing-tree handling is covered by asserting every entry in the tree is
repaired, and the Windows command contract is covered by mocked tests run
in CI.

* fix(lark): harden Windows credential tree against TOCTOU and hard-link races

This replaces the path-based Windows hardening (lstat -> SetFileSecurityW(path) -> iterdir) with a handle-relative walker, so validation, the ACL update, and traversal are bound to the opened object rather than a re-resolved pathname.

Every credential object is opened no-follow; children are enumerated with GetFileInformationByHandleEx(FileFullDirectoryInfo) and opened/created relative to an already-open parent handle (NtOpenFile/NtCreateFile with OBJECT_ATTRIBUTES.RootDirectory), so a pathname swap cannot redirect the walk. Credential directories are opened exclusively (share=0): SetSecurityInfo therefore does not propagate the final owner-only OI|CI DACL into as-yet-unvalidated children, and the namespace is locked for the duration of the walk (concurrent child rename/replacement and hard-link insertion fail with sharing violations). Any file with nNumberOfLinks != 1 is rejected before its security descriptor is touched, so an NTFS hard link to an external file cannot change that file owner/DACL. POSIX keeps the lstat-before-descent walk.

Tests: native regressions for exclusive no-propagation, late hard-link insertion being blocked, mid-walk junction swap being blocked, static hard-link rejection, and both real NTFS junction rejections. Mock seams updated for the handle-relative API, and Windows portability fixes make the suite green on Windows except the known #5116 sandbox-runtime executable-bit failures.

* test(lark): keep the credential-tree symlink assertion portable

The credential-tree symlink rejection is a ValueError; POSIX reports a symlink while the Windows handle-relative walker reports a reparse point. Use a platform-dependent regex so the test passes on Linux/macOS and Windows.

* fix(lark): close remaining credential-tree hardening gaps

Review follow-up for the handle-relative credential-tree walker:
- Stage the transaction snapshot under the owner-only root, copying only config/ and data/.
- Serialize ensure() per-user across threads and processes with a dedicated lock.
- Make the walker iterative so deep trees cannot hit the recursion limit.
- Re-reject a symlinked POSIX root before mkdir; drop the over-strict ancestor-chain check.
- Soften the SetSecurityInfo failure claim; add regressions for each and carry os.SEEK_END in the os stub.

* fix(lark): anchor hardening lock under trusted base and keep POSIX untouched

Follow-up refinements to the credential-tree hardening:
- The per-user hardening lock file now lives directly under the trusted base_dir
  instead of the unverified per-user chain, so it is never written through an
  ancestor that has not yet passed reparse validation.
- ensure() takes the hardening lock only on the Windows branch; POSIX keeps the
  original contract, so no new lock-file side effect.
- Strengthen the ancestor-junction regression (lock not written to the external
  target) and fix two test docstrings to match the parent-first order and the
  no-prior-broadening failure claim.

* fix(lark): anchor credential-operation lock under trusted base on Windows

The per-user credential lock (_lark_credential_lock) created its advisory lock file
under the unverified per-user chain (users/<id>/integrations/.lark-cli.credentials.lock)
before ensure() validated the ancestor chain. On Windows it is now anchored directly
under the trusted paths.base_dir (mirroring the hardening lock), so a junction at
integrations can no longer cause the credential lock to be written into an external
target before reparse validation. POSIX keeps the original location unchanged.

Tests:
- Public-flow regression (start_lark_config -> credential lock -> ensure) uses an empty
  sentinel lock file to prove the old credential-lock path is never opened/written.
- CLI-write re-harden tests restore the POSIX outcome assertion (file tightened to 0600).

...

This commit is contained in:

spud 2026-09-06 08:50:05 +08:00 committed by GitHub

parent cd2633725b

commit 2e85901876

No known key found for this signature in database

GPG Key ID: B5690EEEBB952194

2 changed files with 1936 additions and 109 deletions

Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL

Show Stats Download Patch File Download Diff File Expand all files Collapse all files

948

backend/packages/harness/deerflow/integrations/lark_cli.py

File diff suppressed because it is too large Load Diff

1097

backend/tests/test_lark_cli_integration.py

File diff suppressed because it is too large Load Diff