Skip to content

fix(lark): keep CLI lock directory writable in sandboxes - #4701

Open
Creeper998 wants to merge 1 commit into
bytedance:mainfrom
Creeper998:codex/fix-lark-cli-lock-dir
Open

fix(lark): keep CLI lock directory writable in sandboxes#4701
Creeper998 wants to merge 1 commit into
bytedance:mainfrom
Creeper998:codex/fix-lark-cli-lock-dir

Conversation

@Creeper998

@Creeper998 Creeper998 commented Aug 5, 2026

Copy link
Copy Markdown

Fixes #4700

Why

The managed Lark/Feishu integration fails inside sandboxes when lark-cli creates its config/locks coordination directory. DeerFlow deliberately mounts the credential-bearing config root read-only, so API commands currently stop with a read-only filesystem error even after a successful connection.

This keeps the app secret protected from sandbox writes while restoring the CLI command surface.

What changed

  • Pre-create each user’s config/locks directory with owner-only permissions.
  • Keep /mnt/integrations/lark-cli/config read-only and over-mount only its locks child writable.
  • Preserve the nested mount through local AIO and remote Provisioner Pattern A.
  • Mount config/locks/data only into the broker sidecar for Pattern B; the sandbox still receives no credential mounts.
  • Extend the mount allowlist/limit and add local, remote, and broker regression coverage.
  • Document the read-only parent / writable child trust boundary.

Surface area

  • Frontend UI — page / component / setting / interaction under frontend/
  • Backend API — endpoint / SSE event / request-response shape under backend/app
  • Agents / LangGraph — agent node, graph wiring, langgraph.json, or prompt change
  • Sandboxdocker/ or sandboxed execution
  • Skills — change under skills/
  • Dependencies — new/upgraded entry in backend/pyproject.toml or frontend/package.json
  • Default behavior change — changes existing behavior without the user opting in
  • Docs / tests / CI only — no runtime behavior change

Screenshots / Recording

Not applicable; there is no UI change.

Bug fix verification

  • Test paths: backend/tests/test_aio_sandbox_provider.py, backend/tests/test_lark_cli_integration.py, backend/tests/test_provisioner_mount_contract.py, and backend/tests/test_provisioner_pvc_volumes.py.
  • The new focused tests failed on upstream/main because the lock directory/mount and Provisioner contract did not exist, then passed after the implementation.

Validation

  • pytest tests/test_aio_sandbox_provider.py tests/test_lark_cli_integration.py tests/test_provisioner_mount_contract.py tests/test_provisioner_pvc_volumes.py tests/test_lark_broker.py -q191 passed.
  • Ruff check on all changed backend and test Python files — passed.
  • python -m py_compile docker/provisioner/app.py — passed.
  • Full backend suite — 11,088 passed, 34 skipped, 15 failed. All 15 failures are existing browser/web-fetch tests in this local environment because DNS resolution classifies public test hosts (example.com and GitHub) as private addresses under the SSRF guard; none touches the Lark/sandbox code changed here.

AI assistance

Tool(s) used: Codex

How you used it: Diagnosed the runtime failure, wrote red-first regression tests, implemented the local/Provisioner/broker fix, reviewed the diff, and ran the validation above.

  • I have read and understand every line of this change and take responsibility for it — it is not unreviewed AI output.

@CLAassistant

CLAassistant commented Aug 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added area:docs Documentation and Markdown only area:sandbox Sandboxed execution and docker/ risk:high High risk: backend API, agents, sandbox, auth, deps, CI size/M PR changes 100-300 lines labels Aug 5, 2026

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the lock-directory fix end to end across AIO local sandbox, remote Pattern A (init container), and broker Pattern B (sidecar). The credential-tree, mount, allowlist, MAX_EXTRA_MOUNTS, and Gateway/provisioner contract changes are all consistent; config stays read-only while the empty config/locks subdir is over-mounted writable, and the sandbox trust-boundary docs in README.md / backend/AGENTS.md are updated to match. Test coverage spans all three modes.

One test-robustness suggestion inline (the nested over-mount order isn't asserted). Otherwise this looks good.

assert provisioner_module.LARK_BROKER_SIDECAR_CONFIG_PATH in sidecar_paths
assert provisioner_module.LARK_BROKER_SIDECAR_LOCKS_PATH in sidecar_paths
assert provisioner_module.LARK_BROKER_SIDECAR_DATA_PATH in sidecar_paths
sidecar_mounts = {m.mount_path: m for m in sidecar.volume_mounts}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion (test robustness): this dict comprehension (and the matching sandbox_mounts = {m.mount_path: m ...} in test_runtime_extra_mount_dropped_when_init_container_enabled) discards the volume_mounts list order.

The nested over-mount design — writable config/locks layered on top of read-only config — relies on the parent appearing before the child in the volumeMounts list that kubelet/Docker receive. That ordering comes from the explicit tuple in _build_lark_cli_broker_sidecars (config, locks, data) and from _get_lark_cli_runtime_mounts on the Gateway side. Most runtimes sort mounts by path depth, so this is defense-in-depth rather than a live bug, but the current read-only assertions would still pass if someone reordered the tuple and silently broke the over-mount at runtime.

A cheap order assertion here would document the invariant and catch that, e.g.:

mount_order = [m.mount_path for m in sidecar.volume_mounts]
assert mount_order.index(provisioner_module.LARK_BROKER_SIDECAR_CONFIG_PATH) \
    < mount_order.index(provisioner_module.LARK_BROKER_SIDECAR_LOCKS_PATH)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:docs Documentation and Markdown only area:sandbox Sandboxed execution and docker/ risk:high High risk: backend API, agents, sandbox, auth, deps, CI size/M PR changes 100-300 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(lark): CLI cannot create locks under read-only config mount

3 participants