6 Commits

Author SHA1 Message Date
spud
2e85901876
fix(lark): enforce private ACLs on Windows credential tree (#5141)
* 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).
2026-09-06 08:50:05 +08:00
Daoyuan Li
df57f8e269
fix(lark): preserve app secrets during credential switches (#4820) 2026-09-01 15:37:19 +08:00
Creeper998
17531d7c11
fix(lark): keep CLI lock directory writable in sandboxes (#4701)
* fix(lark): provide writable CLI lock directory

* test(lark): pin nested lock mount ordering
2026-08-10 11:01:14 +08:00
Ryker_Feng
e401ae2d7b
feat(integrations): support switching Lark app credentials (#4703)
* feat(integrations): support switching Lark app credentials

* fix(integrations): harden Lark app switching

* refactor(integrations): simplify Lark switch flow

* fix(integrations): reject superseded Lark flows

* test(integrations): pass Lark flow generation

* fix(integrations): preserve pending Lark flows

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-08-10 07:19:19 +08:00
Ryker_Feng
aacb99cfd2
feat(lark): sidecar credential broker for sandbox lark-cli (Pattern B) (#4501)
* feat(lark): sidecar credential broker for sandbox lark-cli (Pattern B)

Removes the plaintext Lark credential mounts (appSecret + OAuth tokens)
from the sandbox container. A long-running broker sidecar owns lark-cli
and the per-user config/data dirs and serves the command surface over
Pod loopback; the sandbox gets only a forwarding shim on PATH, so the
raw credential files never exist in the sandbox filesystem.

- lark_broker.py: stdlib-only loopback broker (argv passthrough with
  shell=False, server-injected credential env, bounded I/O) + shim
  script constant + install-shim mode.
- docker/lark-cli-broker: init(install-shim) + serve image.
- provisioner: LARK_CLI_BROKER_IMAGE + provision_lark_cli_broker →
  shim init container + lark-cli-broker sidecar (config/data mounted
  sidecar-only); credentials dropped from the sandbox container;
  /api/capabilities reports lark_cli_broker_image. Broker supersedes
  the Pattern A init-container binary when both are configured.
- gateway: lark_cli_env_overlay(broker=True) omits config/data env;
  sandbox_lark_broker_active() TTL-cached mode resolver; broker added
  to sandbox_runtime_mode / readiness and the settings UI.

Opt-in and off by default (empty LARK_CLI_BROKER_IMAGE ⇒ no change).

Closes #4338

* fix(lark): address Pattern B broker review findings (#4501)

Follow-up to the sidecar credential broker addressing the PR #4501 review:

- shim: split the on-PATH lark-cli into a /bin/sh launcher + Python shim body
  so broker mode fails loudly (exit 127, actionable message) instead of ENOEXEC
  when the sandbox image ships no python3; interpreter pinnable via
  DEERFLOW_LARK_BROKER_PYTHON. Launcher bakes in the shim's absolute path since
  $0 is the bare command name when run off PATH.
- broker: drop the dead cwd payload field (broker can't see the sandbox FS) and
  document the command-surface-only / no-file-IO limitation.
- broker: return a structured 500 JSON on unexpected exec errors so the shim
  gets a meaningful message, not an opaque transport failure; set a handler
  socket timeout to bound slow/stuck connections.
- broker: add an opt-in DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS denylist that
  refuses secret-dumping subcommands before spawning the binary, forwarded from
  the provisioner sidecar.
- gateway: tighten the per-bash-call broker probe timeout (1.5s) and cache
  negatives longer (300s) so non-broker remote-provisioner users don't pay a
  latency hit; guard the mode cache with a lock; drop the dead
  _probe_provisioner_lark_cli_init_image wrapper.
- docs: remove the broken design-doc link from the broker README.

Adds tests for launcher python resolution, cwd omission, denylist enforcement,
500-on-error, hot-path probe timeout + negative caching, and provisioner
denylist-env wiring.
2026-07-28 22:54:44 +08:00
Ryker_Feng
7aa314b4c1
feat: add Lark CLI integration (#3971)
* feat: add lark cli integration

* fix: polish lark integration actions

* feat: support lark incremental permissions

* fix: detect lark authorization completion

* fix: harden lark integration install

* feat: expand lark auth scopes and reuse host auth in sandbox

Default lark auth to least-privilege (recommend=false, base sign-in only)
and expose the full set of lark-cli --domain business domains as native
--domain grants instead of a 4-domain read-only mapping. Resolve the
skill pack from the latest larksuite/cli GitHub release at install time
with content-hash integrity, and surface version/runtime drift in status.

Share the per-user lark-cli config/data profile between the Gateway
Settings auth flow and agent conversations by mounting the integration
dirs into the AIO sandbox and injecting the matching env for lark-cli
commands, with an allowlisted extra_mounts path in the provisioner/K8s
backend and traversal guards on integration paths.

* style: fix lint issues from ruff and prettier

Sort imports in the provisioner PVC test and re-wrap two long i18n
description strings to satisfy backend ruff and frontend prettier CI.

* fix(lark): address managed integration review feedback

* fix(frontend): stabilize integrations settings e2e

* test(sandbox): isolate remote backend legacy visibility check

* test: fix backend unit failures after merge

* Harden Lark integration review fixes

* Format Lark integration E2E test

* fix(lark): harden sandbox credential exposure and status disclosure

Address willem_bd's security review on PR #3971:

- Mount the per-user lark-cli config dir (long-lived appSecret) read-only
  into the AIO sandbox; only the refreshable-token data dir stays writable.
- Redact host filesystem paths (install_path, cli.path) from
  GET /lark/status and the config/auth complete responses for non-admin
  callers, fail-closed on any auth error.
- Document the npm postinstall trade-off (--ignore-scripts is not viable
  because @larksuite/cli fetches its platform binary in postinstall).
- Document the sandbox credential trust boundary in AGENTS.md and README,
  pointing at the sidecar-broker follow-up (#4338).

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-07-26 08:09:17 +08:00