diff --git a/README.md b/README.md index e7afc2f71..107041342 100644 --- a/README.md +++ b/README.md @@ -791,8 +791,10 @@ set `DEER_FLOW_LARK_CLI_SANDBOX_RUNTIME_DIR` to that directory. > **Sandbox trust boundary:** the browser never receives the Lark app secret, but > agent conversations run `lark-cli` inside the sandbox, so the per-user > credential directories are mounted into it: `config` (holding the long-lived -> `appSecret`) is mounted **read-only** and `data` (refreshable OAuth tokens) -> writable. Both remain *readable* by any process the agent runs there, so code +> `appSecret`) is mounted **read-only**, its otherwise empty `config/locks` +> subdirectory is over-mounted writable for `lark-cli` coordination files, and +> `data` (refreshable OAuth tokens) is writable. The credential-bearing config +> and data mounts remain *readable* by any process the agent runs there, so code > reached via prompt injection in a tool result could read them. Treat the > sandbox as inside the Lark credential trust boundary until the sidecar > credential-broker follow-up removes these mounts from sandbox execution. diff --git a/backend/AGENTS.md b/backend/AGENTS.md index a97d53fd4..744a504cc 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -877,7 +877,7 @@ E2B output sync records remote file versions and actual host file metadata in a - `skills/describe.py` — `build_describe_skill_tool(catalog)` builds the `describe_skill` tool as a closure; `build_skill_search_setup(skills, enabled, ...)` produces a `SkillSearchSetup(describe_skill_tool, skill_names)` that is wired into both the LangGraph agent factory (`agent.py`) and the embedded client (`client.py`). - **Slash activation**: `/skill-name task` loads that enabled skill's `SKILL.md` for the current model call only. The resolver rejects leading whitespace, missing separators, reserved channel commands (`/new`, `/help`, `/bootstrap`, `/status`, `/models`, `/memory`, `/goal`), disabled skills, and skills outside a custom agent's whitelist. - **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory -- **Managed integrations**: Lark/Feishu CLI support installs one global official `lark-*` pack as read-only `SkillCategory.INTEGRATION` entries under `/mnt/skills/integrations/lark-cli/...`; enabled flags, app configuration, and OAuth data remain per-user. Install resolves the newest `larksuite/cli` release from GitHub (`releases/latest`) at install time (falling back to a bottom-line pinned version if the lookup fails) rather than hard-coding the pack version; integrity relies on the official host + structural archive guards + a recorded hash of the effective installed tree after shared guidance injection (not a pinned archive-byte SHA, which GitHub does not keep stable). The Gateway image still installs a pinned `@larksuite/cli` binary, so `get_lark_integration_status` surfaces `latest_available_version` and `runtime_version_mismatch` for the UI. AIO installs additionally verify and publish official Linux amd64/arm64 binaries under `{DEER_FLOW_HOME}/integrations/lark-cli/sandbox-cli`, mounted read-only at `/mnt/integrations/lark-cli/runtime`; `/mnt/integrations/lark-cli/config` (app credentials, incl. the long-lived `appSecret`) is mounted **read-only** into the sandbox and `/mnt/integrations/lark-cli/data` (refreshable OAuth tokens) stays writable, both mapping to owner-only per-user credential directories. **Sandbox trust boundary:** those two dirs are still *readable* by arbitrary sandbox processes (the agent's `bash` tool, or code reached via prompt-injection in a tool result), so the app secret and tokens are exposed to sandbox-side code even though they never reach the browser — the read-only config mount only prevents in-sandbox tampering, not read/exfiltration. The sidecar credential-broker (Pattern B, issue #4338) is the fix that removes these plaintext mounts from sandbox execution: set `LARK_CLI_BROKER_IMAGE` on the provisioner (see `docker/lark-cli-broker/`) and the Gateway sends `provision_lark_cli_broker` on sandbox create. The provisioner then runs a `lark-cli-broker` sidecar that owns the per-user `config`/`data` (mounted into the **sidecar only**, at `/var/lark/{config,data}`) and serves the `lark-cli` command surface on Pod loopback (`http://127.0.0.1:8788`); a shim init container (`install-shim`) writes a forwarding `lark-cli` into the shared runtime `emptyDir`, so the sandbox gets `DEERFLOW_LARK_BROKER_URL` + a shim on PATH but **no** credential files. The on-PATH `bin/lark-cli` is a `/bin/sh` launcher that resolves a Python 3 interpreter and execs the Python shim body (`bin/lark-cli-shim.py`) beside it, so broker mode does not silently ENOEXEC on a sandbox image without a `#!/usr/bin/env python3`-resolvable interpreter — it fails loudly (exit 127, actionable message) and can be pinned with `DEERFLOW_LARK_BROKER_PYTHON`. The broker runs `lark-cli` in the sidecar's cwd and cannot see the sandbox filesystem, so cwd is intentionally **not** forwarded and file-I/O subcommands relative to the sandbox cwd are unsupported (command surface only). An optional `DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS` denylist (comma-separated command prefixes, forwarded from the provisioner) lets the broker refuse secret-dumping subcommands before spawning the binary. `lark_cli_env_overlay(broker=True)` therefore omits `LARKSUITE_CLI_CONFIG_DIR`/`DATA_DIR`; `sandbox_lark_broker_active()` (TTL-cached provisioner `/api/capabilities` probe, tight timeout + longer negative caching on the bash hot path) selects broker vs. binary mode for both the bash env overlay and status. `DEER_FLOW_LARK_CLI_SANDBOX_RUNTIME_DIR` supplies a validated, symlink-free pre-staged runtime for air-gapped deployments. For the remote provisioner (K8s), the runtime binary is otherwise provisioned by an optional init container + shared `emptyDir` (Pattern A): set `LARK_CLI_INIT_IMAGE` on the provisioner (see `docker/lark-cli-init/`) and the Gateway sends `provision_lark_cli_runtime` on sandbox create once the pack is installed, so remote installs skip the Gateway-side GitHub download entirely. Broker (Pattern B) supersedes the init-container binary (Pattern A) when both images are configured. `get_lark_integration_status(check_runtime=True)` surfaces `sandbox_runtime_mode` (`none` / `gateway-download` / `init-container` / `broker`) and `sandbox_runtime_ready` (remote modes read the provisioner `GET /api/capabilities`: `lark_cli_init_image` / `lark_cli_broker_image`) so a green UI can't hide a chat-time `lark-cli: command not found`. Cheap status probes are explicitly not live-verified; users authorize or reconnect through the browser device-flow endpoints instead of running terminal commands. +- **Managed integrations**: Lark/Feishu CLI support installs one global official `lark-*` pack as read-only `SkillCategory.INTEGRATION` entries under `/mnt/skills/integrations/lark-cli/...`; enabled flags, app configuration, and OAuth data remain per-user. Install resolves the newest `larksuite/cli` release from GitHub (`releases/latest`) at install time (falling back to a bottom-line pinned version if the lookup fails) rather than hard-coding the pack version; integrity relies on the official host + structural archive guards + a recorded hash of the effective installed tree after shared guidance injection (not a pinned archive-byte SHA, which GitHub does not keep stable). The Gateway image still installs a pinned `@larksuite/cli` binary, so `get_lark_integration_status` surfaces `latest_available_version` and `runtime_version_mismatch` for the UI. AIO installs additionally verify and publish official Linux amd64/arm64 binaries under `{DEER_FLOW_HOME}/integrations/lark-cli/sandbox-cli`, mounted read-only at `/mnt/integrations/lark-cli/runtime`; `/mnt/integrations/lark-cli/config` (app credentials, incl. the long-lived `appSecret`) is mounted **read-only** into the sandbox, its empty `config/locks` subdirectory is over-mounted writable for `lark-cli` coordination files, and `/mnt/integrations/lark-cli/data` (refreshable OAuth tokens) stays writable, all mapping to owner-only per-user directories. **Sandbox trust boundary:** the credential-bearing config and data dirs are still *readable* by arbitrary sandbox processes (the agent's `bash` tool, or code reached via prompt-injection in a tool result), so the app secret and tokens are exposed to sandbox-side code even though they never reach the browser — the read-only config mount only prevents in-sandbox tampering, not read/exfiltration. The sidecar credential-broker (Pattern B, issue #4338) is the fix that removes these plaintext mounts from sandbox execution: set `LARK_CLI_BROKER_IMAGE` on the provisioner (see `docker/lark-cli-broker/`) and the Gateway sends `provision_lark_cli_broker` on sandbox create. The provisioner then runs a `lark-cli-broker` sidecar that owns the per-user `config`/`config/locks`/`data` mounts (mounted into the **sidecar only**, at `/var/lark/{config,config/locks,data}` with only the nested locks mount writable) and serves the `lark-cli` command surface on Pod loopback (`http://127.0.0.1:8788`); a shim init container (`install-shim`) writes a forwarding `lark-cli` into the shared runtime `emptyDir`, so the sandbox gets `DEERFLOW_LARK_BROKER_URL` + a shim on PATH but **no** credential files. The on-PATH `bin/lark-cli` is a `/bin/sh` launcher that resolves a Python 3 interpreter and execs the Python shim body (`bin/lark-cli-shim.py`) beside it, so broker mode does not silently ENOEXEC on a sandbox image without a `#!/usr/bin/env python3`-resolvable interpreter — it fails loudly (exit 127, actionable message) and can be pinned with `DEERFLOW_LARK_BROKER_PYTHON`. The broker runs `lark-cli` in the sidecar's cwd and cannot see the sandbox filesystem, so cwd is intentionally **not** forwarded and file-I/O subcommands relative to the sandbox cwd are unsupported (command surface only). An optional `DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS` denylist (comma-separated command prefixes, forwarded from the provisioner) lets the broker refuse secret-dumping subcommands before spawning the binary. `lark_cli_env_overlay(broker=True)` therefore omits `LARKSUITE_CLI_CONFIG_DIR`/`DATA_DIR`; `sandbox_lark_broker_active()` (TTL-cached provisioner `/api/capabilities` probe, tight timeout + longer negative caching on the bash hot path) selects broker vs. binary mode for both the bash env overlay and status. `DEER_FLOW_LARK_CLI_SANDBOX_RUNTIME_DIR` supplies a validated, symlink-free pre-staged runtime for air-gapped deployments. For the remote provisioner (K8s), the runtime binary is otherwise provisioned by an optional init container + shared `emptyDir` (Pattern A): set `LARK_CLI_INIT_IMAGE` on the provisioner (see `docker/lark-cli-init/`) and the Gateway sends `provision_lark_cli_runtime` on sandbox create once the pack is installed, so remote installs skip the Gateway-side GitHub download entirely. Broker (Pattern B) supersedes the init-container binary (Pattern A) when both images are configured. `get_lark_integration_status(check_runtime=True)` surfaces `sandbox_runtime_mode` (`none` / `gateway-download` / `init-container` / `broker`) and `sandbox_runtime_ready` (remote modes read the provisioner `GET /api/capabilities`: `lark_cli_init_image` / `lark_cli_broker_image`) so a green UI can't hide a chat-time `lark-cli: command not found`. Cheap status probes are explicitly not live-verified; users authorize or reconnect through the browser device-flow endpoints instead of running terminal commands. - **SkillScan**: `packages/harness/deerflow/skills/skillscan/` is the native deterministic scanner for `.skill` archives and agent-managed skill writes. It runs offline before the LLM scanner, emits structured findings (`rule_id`, `severity`, `file`, `line`, `message`, `remediation`, redacted `evidence` — category/analyzer are encoded in the `rule_id` prefix), blocks `CRITICAL`, and passes warning findings into `scan_skill_content()`. `scan_archive_preflight()` / `scan_skill_dir()` are pure sync functions (dispatch off the event loop); `enforce_static_scan()` applies the blocking policy and the `skill_scan.enabled` kill switch. The Python instance-client signal deliberately follows only a one-level, same-scope evidence chain (PR #4265 review): a proven imported constructor bound to a simple name, optional name-to-name alias propagation, rebinding invalidation, and a constructor-supported outbound method or context-manager use; bare canonical-looking names never fall back to module identity. Nested scopes never inherit client handles and inherit only constructor aliases proven stable by a binding-only enclosing-scope prepass. Comprehensions, walrus-bearing statements, annotations, executable expressions inside complex binding targets, unsupported operations, and ambiguous flows produce no finding from this signal; skipped constructs invalidate all names they may bind, while representative false negatives are pinned by `test_python_declared_false_negatives_stay_unreported`. Compound bodies are walked from isolated copies so wrapping code in `if True:` is not a bypass, while copied scope entries, binding-only prepasses, and AST visits consume a deterministic work budget and the walk stops after its first sink. Budget or recursion exhaustion skips only this best-effort signal and retains deterministic findings already collected for the file. Do not add Semgrep/OpenGrep or YAML rule-engine dependencies to the core path; Phase 1 rule specs live in Python constants next to their analyzers in `skillscan/orchestrator.py`. - **Skill Review Core**: `packages/harness/deerflow/skills/review/` provides read-only package snapshots, deterministic facts, resource/eval analysis, report rendering, and the CLI (`python -m deerflow.skills.review.cli`). It reuses the shared frontmatter helper and SkillScan; it must not import `app.*`, execute target scripts, install dependencies, or call networks. JSON contracts live in `contracts/skill_review/`. The `review_skill_package` built-in tool labels results with `review_subject_entry` and never `skill_context_entry`, so reviewing a target does not activate it, bind its `required-secrets`, or apply its `allowed-tools`. Its model-visible `ToolMessage.content` is a compact JSON payload with untrusted control tags neutralized; the full raw review payload, including Markdown renders, stays in `ToolMessage.artifact`. CI should run the CLI with `--fail-on error --fail-on-incomplete` so blocker/error findings and truncated/not-assessed packages fail the gate. The public `skills/public/skill-reviewer` skill owns semantic readiness review and suggestions only; mutation and runtime experiments remain owned by `skill-creator`. diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py index cd7d4ab97..dc44aab7b 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py @@ -40,7 +40,7 @@ from deerflow.community.warm_pool_lifecycle import ( from deerflow.config import get_app_config from deerflow.config.paths import VIRTUAL_PATH_PREFIX, get_paths, join_host_path from deerflow.integrations.lark_cli import INTEGRATION_ID as LARK_CLI_INTEGRATION_ID -from deerflow.integrations.lark_cli import LARK_CLI_SANDBOX_CONFIG_DIR, LARK_CLI_SANDBOX_DATA_DIR, LARK_CLI_SANDBOX_RUNTIME_DIR, ensure_lark_cli_credential_tree, lark_skills_installed +from deerflow.integrations.lark_cli import LARK_CLI_SANDBOX_CONFIG_DIR, LARK_CLI_SANDBOX_DATA_DIR, LARK_CLI_SANDBOX_LOCKS_DIR, LARK_CLI_SANDBOX_RUNTIME_DIR, ensure_lark_cli_credential_tree, lark_skills_installed from deerflow.runtime.user_context import get_effective_user_id from deerflow.sandbox.sandbox import Sandbox from deerflow.sandbox.sandbox_provider import SandboxProvider @@ -1019,8 +1019,11 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): ``lark-cli config init`` on the Gateway, never in-sandbox), so it is mounted **read-only**: sandbox processes only need to read it, and a read-only bind stops a compromised agent from tampering with or - replacing the app credentials. The ``data`` dir holds refreshable OAuth - tokens that ``lark-cli auth`` updates in-sandbox, so it stays writable. + replacing the app credentials. Newer ``lark-cli`` versions coordinate + API calls through ``config/locks``, so that empty subdirectory is + over-mounted writable without exposing the rest of ``config`` to + writes. The ``data`` dir holds refreshable OAuth tokens that + ``lark-cli auth`` updates in-sandbox, so it stays writable. This is defense-in-depth only — both dirs remain readable to arbitrary sandbox processes until the auth-proxy follow-up (issue #4338) lands. See the sandbox trust-boundary note in ``backend/AGENTS.md``. @@ -1029,8 +1032,10 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): paths = get_paths() effective_user_id = AioSandboxProvider._effective_acquire_user_id(user_id) ensure_lark_cli_credential_tree(effective_user_id, paths=paths) + config_dir = paths.host_user_integration_config_dir(effective_user_id, LARK_CLI_INTEGRATION_ID) mounts = [ - (paths.host_user_integration_config_dir(effective_user_id, LARK_CLI_INTEGRATION_ID), LARK_CLI_SANDBOX_CONFIG_DIR, True), + (config_dir, LARK_CLI_SANDBOX_CONFIG_DIR, True), + (join_host_path(config_dir, "locks"), LARK_CLI_SANDBOX_LOCKS_DIR, False), (paths.host_user_integration_data_dir(effective_user_id, LARK_CLI_INTEGRATION_ID), LARK_CLI_SANDBOX_DATA_DIR, False), ] runtime_dir = paths.base_dir / "integrations" / LARK_CLI_INTEGRATION_ID / "sandbox-cli" diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py b/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py index f34415907..8fdee1929 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py @@ -34,6 +34,7 @@ _PROVISIONER_EXTRA_MOUNT_PATHS = { "/mnt/skills/custom", "/mnt/skills/integrations", "/mnt/integrations/lark-cli/config", + "/mnt/integrations/lark-cli/config/locks", "/mnt/integrations/lark-cli/data", "/mnt/integrations/lark-cli/runtime", } @@ -54,15 +55,17 @@ def _provisioner_extra_mounts_payload( When ``provision_lark_cli_runtime`` is set, the provisioner supplies the lark-cli runtime via an init container + emptyDir, so the runtime extra mount is dropped here to avoid a colliding hostPath/PVC mount at the same path. The - per-user config/data credential mounts are still forwarded (they are mounted - into the sandbox in Pattern A). + per-user config/locks/data mounts are still forwarded (they are mounted into + the sandbox in Pattern A). The config root remains read-only while its + nested locks mount is writable for lark-cli's coordination files. When ``provision_lark_cli_broker`` is set (Pattern B, issue #4338), the provisioner runs a broker sidecar that holds the credentials, so the - config/data mounts are **forwarded** (the provisioner wires them into the - sidecar, not the sandbox) while the runtime mount is dropped. Nothing changes - in this payload beyond keeping config/data available for the provisioner to - place; the runtime entry is dropped in both modes. + config/locks/data mounts are **forwarded** (the provisioner wires them into + the sidecar, not the sandbox) while the runtime mount is dropped. Nothing + changes in this payload beyond keeping those credential-related mounts + available for the provisioner to place; the runtime entry is dropped in + both modes. """ if not extra_mounts: return [] diff --git a/backend/packages/harness/deerflow/integrations/lark_cli.py b/backend/packages/harness/deerflow/integrations/lark_cli.py index e8b16e773..7fd371832 100644 --- a/backend/packages/harness/deerflow/integrations/lark_cli.py +++ b/backend/packages/harness/deerflow/integrations/lark_cli.py @@ -106,6 +106,7 @@ LARK_CLI_MAX_EXTRACTED_BYTES = 256 * 1024 * 1024 LARK_CLI_MAX_RUNTIME_ASSET_BYTES = 128 * 1024 * 1024 LARK_CLI_MANIFEST_FILE = ".deerflow-lark-cli-manifest.json" LARK_CLI_SANDBOX_CONFIG_DIR = "/mnt/integrations/lark-cli/config" +LARK_CLI_SANDBOX_LOCKS_DIR = f"{LARK_CLI_SANDBOX_CONFIG_DIR}/locks" LARK_CLI_SANDBOX_DATA_DIR = "/mnt/integrations/lark-cli/data" LARK_CLI_SANDBOX_RUNTIME_DIR = "/mnt/integrations/lark-cli/runtime" LARK_CLI_LINUX_ARCHES = ("amd64", "arm64") @@ -309,7 +310,7 @@ def ensure_lark_cli_credential_tree(user_id: str, *, paths: Paths | None = None) raise ValueError(f"Lark CLI credential path must not be a symlink: {root}") root.mkdir(parents=True, exist_ok=True, mode=0o700) root.chmod(0o700) - for required in (root / "config", root / "data"): + for required in (root / "config", root / "config" / "locks", root / "data"): if required.is_symlink(): raise ValueError(f"Lark CLI credential path must not be a symlink: {required}") required.mkdir(parents=True, exist_ok=True, mode=0o700) diff --git a/backend/tests/test_aio_sandbox_provider.py b/backend/tests/test_aio_sandbox_provider.py index 3fabd9757..f4a2942dc 100644 --- a/backend/tests/test_aio_sandbox_provider.py +++ b/backend/tests/test_aio_sandbox_provider.py @@ -183,17 +183,24 @@ def test_get_lark_cli_runtime_mounts_uses_user_auth_dirs(tmp_path, monkeypatch): runtime_dir.mkdir(parents=True) mounts = aio_mod.AioSandboxProvider._get_lark_cli_runtime_mounts(user_id="alice") + mount_order = [container_path for _host_path, container_path, _read_only in mounts] container_paths = {container_path: (host_path, read_only) for host_path, container_path, read_only in mounts} assert container_paths[lark_cli.LARK_CLI_SANDBOX_CONFIG_DIR] == ( str(tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "config"), True, ) + assert container_paths[f"{lark_cli.LARK_CLI_SANDBOX_CONFIG_DIR}/locks"] == ( + str(tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "config" / "locks"), + False, + ) + assert mount_order.index(lark_cli.LARK_CLI_SANDBOX_CONFIG_DIR) < mount_order.index(lark_cli.LARK_CLI_SANDBOX_LOCKS_DIR) assert container_paths[lark_cli.LARK_CLI_SANDBOX_DATA_DIR] == ( str(tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "data"), False, ) assert stat.S_IMODE((tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "config").stat().st_mode) == 0o700 + assert stat.S_IMODE((tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "config" / "locks").stat().st_mode) == 0o700 assert stat.S_IMODE((tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "data").stat().st_mode) == 0o700 assert container_paths["/mnt/integrations/lark-cli/runtime"] == ( str(runtime_dir), @@ -254,12 +261,14 @@ def test_get_extra_mounts_provisioner_payload_has_unique_container_paths(tmp_pat assert "/mnt/skills/custom" in container_paths assert "/mnt/skills/integrations" in container_paths assert lark_cli.LARK_CLI_SANDBOX_CONFIG_DIR in container_paths + assert lark_cli.LARK_CLI_SANDBOX_LOCKS_DIR in container_paths assert lark_cli.LARK_CLI_SANDBOX_DATA_DIR in container_paths assert lark_cli.LARK_CLI_SANDBOX_RUNTIME_DIR in container_paths payload = remote_backend._provisioner_extra_mounts_payload(mounts) payload_paths = [str(item["container_path"]) for item in payload] assert len(payload_paths) == len(set(payload_paths)) + assert payload_paths.index(lark_cli.LARK_CLI_SANDBOX_CONFIG_DIR) < payload_paths.index(lark_cli.LARK_CLI_SANDBOX_LOCKS_DIR) provisioner_module.DEER_FLOW_HOST_BASE_DIR = str(home) validated = provisioner_module._validated_extra_mounts([provisioner_module.ExtraMount(**item) for item in payload]) @@ -271,6 +280,7 @@ def test_get_extra_mounts_provisioner_payload_has_unique_container_paths(tmp_pat "/mnt/skills/custom", "/mnt/skills/integrations", lark_cli.LARK_CLI_SANDBOX_CONFIG_DIR, + lark_cli.LARK_CLI_SANDBOX_LOCKS_DIR, lark_cli.LARK_CLI_SANDBOX_DATA_DIR, lark_cli.LARK_CLI_SANDBOX_RUNTIME_DIR, } diff --git a/backend/tests/test_lark_cli_integration.py b/backend/tests/test_lark_cli_integration.py index e1bef5f74..6491313ac 100644 --- a/backend/tests/test_lark_cli_integration.py +++ b/backend/tests/test_lark_cli_integration.py @@ -1073,6 +1073,7 @@ def test_lark_cli_env_hardens_existing_credential_tree(monkeypatch, tmp_path) -> lark_cli.lark_cli_env_overlay("alice") assert stat.S_IMODE(config_dir.stat().st_mode) == 0o700 + assert stat.S_IMODE((config_dir / "locks").stat().st_mode) == 0o700 assert stat.S_IMODE(data_dir.stat().st_mode) == 0o700 assert stat.S_IMODE(secret_file.stat().st_mode) == 0o600 assert stat.S_IMODE(token_file.stat().st_mode) == 0o600 diff --git a/backend/tests/test_provisioner_mount_contract.py b/backend/tests/test_provisioner_mount_contract.py index 403b890e7..d074a3d89 100644 --- a/backend/tests/test_provisioner_mount_contract.py +++ b/backend/tests/test_provisioner_mount_contract.py @@ -25,4 +25,5 @@ def test_gateway_and_provisioner_extra_mount_contracts_match() -> None: assert gateway_paths == provisioner_paths assert "/mnt/integrations/lark-cli/runtime" in gateway_paths - assert _literal_assignment(provisioner_path, "MAX_EXTRA_MOUNTS") == 9 + assert "/mnt/integrations/lark-cli/config/locks" in gateway_paths + assert _literal_assignment(provisioner_path, "MAX_EXTRA_MOUNTS") == 10 diff --git a/backend/tests/test_provisioner_pvc_volumes.py b/backend/tests/test_provisioner_pvc_volumes.py index 656654d12..43b8d7663 100644 --- a/backend/tests/test_provisioner_pvc_volumes.py +++ b/backend/tests/test_provisioner_pvc_volumes.py @@ -538,6 +538,11 @@ class TestLarkCliInitContainer: provisioner_module.ExtraMount( host_path="/state/users/alice/integrations/lark-cli/config", container_path="/mnt/integrations/lark-cli/config", + read_only=True, + ), + provisioner_module.ExtraMount( + host_path="/state/users/alice/integrations/lark-cli/config/locks", + container_path="/mnt/integrations/lark-cli/config/locks", read_only=False, ), provisioner_module.ExtraMount( @@ -555,14 +560,18 @@ class TestLarkCliInitContainer: provision_lark_cli_runtime=True, ) - # The credential config mount stays; the hostPath runtime extra mount is - # replaced by the emptyDir supplied by the init container (so the runtime - # path is not backed by an extra-* hostPath volume). + # The read-only credential config and nested writable locks mounts stay; + # the hostPath runtime extra mount is replaced by the emptyDir supplied + # by the init container (so the runtime path is not backed by an extra-* + # hostPath volume). runtime_mounts = [m for m in pod.spec.containers[0].volume_mounts if m.mount_path == "/mnt/integrations/lark-cli/runtime"] assert len(runtime_mounts) == 1 assert runtime_mounts[0].name == provisioner_module.LARK_CLI_RUNTIME_VOLUME_NAME - mount_paths = {m.mount_path for m in pod.spec.containers[0].volume_mounts} - assert "/mnt/integrations/lark-cli/config" in mount_paths + sandbox_mount_order = [m.mount_path for m in pod.spec.containers[0].volume_mounts] + sandbox_mounts = {m.mount_path: m for m in pod.spec.containers[0].volume_mounts} + assert sandbox_mounts["/mnt/integrations/lark-cli/config"].read_only is True + assert sandbox_mounts["/mnt/integrations/lark-cli/config/locks"].read_only is False + assert sandbox_mount_order.index("/mnt/integrations/lark-cli/config") < sandbox_mount_order.index("/mnt/integrations/lark-cli/config/locks") class TestLarkCliBrokerSidecar: @@ -576,6 +585,11 @@ class TestLarkCliBrokerSidecar: container_path="/mnt/integrations/lark-cli/config", read_only=True, ), + provisioner_module.ExtraMount( + host_path="/state/users/alice/integrations/lark-cli/config/locks", + container_path="/mnt/integrations/lark-cli/config/locks", + read_only=False, + ), provisioner_module.ExtraMount( host_path="/state/users/alice/integrations/lark-cli/data", container_path="/mnt/integrations/lark-cli/data", @@ -635,15 +649,22 @@ class TestLarkCliBrokerSidecar: assert sidecar.image == "deer-flow/lark-cli-broker:v1.0.65" assert sidecar.args == ["serve"] # Credentials mounted into the sidecar only. - sidecar_paths = {m.mount_path for m in sidecar.volume_mounts} + sidecar_mount_order = [m.mount_path for m in sidecar.volume_mounts] + sidecar_mounts = {m.mount_path: m for m in sidecar.volume_mounts} + sidecar_paths = set(sidecar_mounts) 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 + assert sidecar_mounts[provisioner_module.LARK_BROKER_SIDECAR_CONFIG_PATH].read_only is True + assert sidecar_mounts[provisioner_module.LARK_BROKER_SIDECAR_LOCKS_PATH].read_only is False + assert sidecar_mount_order.index(provisioner_module.LARK_BROKER_SIDECAR_CONFIG_PATH) < sidecar_mount_order.index(provisioner_module.LARK_BROKER_SIDECAR_LOCKS_PATH) # Sandbox container: runtime shim mount + broker URL env, NO config/data. sandbox = pod.spec.containers[0] sandbox_paths = {m.mount_path for m in sandbox.volume_mounts} assert provisioner_module.LARK_CLI_RUNTIME_CONTAINER_PATH in sandbox_paths assert "/mnt/integrations/lark-cli/config" not in sandbox_paths + assert "/mnt/integrations/lark-cli/config/locks" not in sandbox_paths assert "/mnt/integrations/lark-cli/data" not in sandbox_paths env = {e.name: e.value for e in (sandbox.env or [])} assert env.get("DEERFLOW_LARK_BROKER_URL") == provisioner_module.LARK_BROKER_URL diff --git a/docker/provisioner/app.py b/docker/provisioner/app.py index 1a6c81367..071e6d8b2 100644 --- a/docker/provisioner/app.py +++ b/docker/provisioner/app.py @@ -70,19 +70,23 @@ LARK_CLI_RUNTIME_VOLUME_NAME = "lark-cli-runtime" # Optional "lark-cli broker" image (Pattern B, issue #4338). When set, sandbox # Pods requesting the broker get an init container that stages a shim + a # long-running broker sidecar that holds the credentials, instead of mounting the -# plaintext config/data credential dirs into the sandbox container. Empty ⇒ broker -# off (Pattern A / legacy behavior). Broker supersedes Pattern A when both are set. +# plaintext config/locks/data credential dirs into the sandbox container. Empty ⇒ +# broker off (Pattern A / legacy behavior). Broker supersedes Pattern A when both +# are set. LARK_CLI_BROKER_IMAGE = os.environ.get("LARK_CLI_BROKER_IMAGE", "") # Optional comma-separated lark-cli subcommand denylist forwarded to the broker # sidecar (issue #4338 hardening). Empty ⇒ no subcommand is blocked. See the # broker README's "subcommand denylist" section. LARK_CLI_BROKER_DENY_SUBCOMMANDS = os.environ.get("DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS", "") LARK_CLI_CONFIG_CONTAINER_PATH = "/mnt/integrations/lark-cli/config" +LARK_CLI_LOCKS_CONTAINER_PATH = f"{LARK_CLI_CONFIG_CONTAINER_PATH}/locks" LARK_CLI_DATA_CONTAINER_PATH = "/mnt/integrations/lark-cli/data" # Where the broker sidecar reads the per-user credentials (sidecar-only paths). LARK_BROKER_SIDECAR_CONFIG_PATH = "/var/lark/config" +LARK_BROKER_SIDECAR_LOCKS_PATH = f"{LARK_BROKER_SIDECAR_CONFIG_PATH}/locks" LARK_BROKER_SIDECAR_DATA_PATH = "/var/lark/data" LARK_BROKER_CONFIG_VOLUME_NAME = "lark-cli-config" +LARK_BROKER_LOCKS_VOLUME_NAME = "lark-cli-locks" LARK_BROKER_DATA_VOLUME_NAME = "lark-cli-data" LARK_BROKER_URL = "http://127.0.0.1:8788" THREADS_HOST_PATH = os.environ.get("THREADS_HOST_PATH", "/.deer-flow/threads") @@ -103,12 +107,13 @@ if SANDBOX_SERVICE_TYPE not in {"NodePort", "ClusterIP"}: SAFE_THREAD_ID_PATTERN = r"^[A-Za-z0-9_-]{1,64}$" SAFE_USER_ID_PATTERN = r"^[A-Za-z0-9_\-]+$" DEFAULT_USER_ID = "default" -MAX_EXTRA_MOUNTS = 9 +MAX_EXTRA_MOUNTS = 10 ALLOWED_EXTRA_MOUNT_PATHS = { "/mnt/acp-workspace", "/mnt/skills/custom", "/mnt/skills/integrations", "/mnt/integrations/lark-cli/config", + "/mnt/integrations/lark-cli/config/locks", "/mnt/integrations/lark-cli/data", "/mnt/integrations/lark-cli/runtime", } @@ -231,18 +236,20 @@ def _runtime_provided_extra_mounts( Pattern A (init container + emptyDir) provides ``/mnt/integrations/lark-cli/runtime``, so a hostPath/PVC mount at the same - path would collide — it is dropped, leaving the per-user ``config`` / ``data`` - credential mounts intact. + path would collide — it is dropped, leaving the per-user ``config`` / + ``config/locks`` / ``data`` mounts intact. The nested locks mount is writable + so lark-cli can coordinate API calls while the config root remains read-only. - Pattern B (broker sidecar) additionally moves the ``config`` / ``data`` - credential mounts off the *sandbox* container and into the sidecar, so those - are dropped here too — the sandbox never sees plaintext credentials. + Pattern B (broker sidecar) additionally moves all three mounts off the + *sandbox* container and into the sidecar, so those are dropped here too — + the sandbox never sees plaintext credentials. """ dropped: set[str] = set() if _lark_cli_broker_enabled(provision_lark_cli_broker): dropped = { LARK_CLI_RUNTIME_CONTAINER_PATH, LARK_CLI_CONFIG_CONTAINER_PATH, + LARK_CLI_LOCKS_CONTAINER_PATH, LARK_CLI_DATA_CONTAINER_PATH, } elif _lark_cli_runtime_enabled(provision_lark_cli_runtime): @@ -253,15 +260,19 @@ def _runtime_provided_extra_mounts( def _lark_broker_credential_mounts(extra_mounts: list["ExtraMount"] | None) -> dict[str, "ExtraMount"]: - """Extract the config/data credential mounts the broker sidecar needs. + """Extract the config/locks/data mounts the broker sidecar needs. Keyed by container path so the caller can wire each into the sidecar's fixed - ``/var/lark/{config,data}`` paths. + ``/var/lark/{config,config/locks,data}`` paths. """ result: dict[str, ExtraMount] = {} for mount in _validated_extra_mounts(extra_mounts): normalized = posixpath.normpath(mount.container_path) - if normalized in (LARK_CLI_CONFIG_CONTAINER_PATH, LARK_CLI_DATA_CONTAINER_PATH): + if normalized in ( + LARK_CLI_CONFIG_CONTAINER_PATH, + LARK_CLI_LOCKS_CONTAINER_PATH, + LARK_CLI_DATA_CONTAINER_PATH, + ): result[normalized] = mount return result @@ -590,11 +601,12 @@ def _build_volumes( empty_dir=k8s_client.V1EmptyDirVolumeSource(), ) ) - # Pattern B: the config/data credential volumes go to the broker sidecar only. + # Pattern B: config/locks/data volumes go to the broker sidecar only. if _lark_cli_broker_enabled(provision_lark_cli_broker): credential_mounts = _lark_broker_credential_mounts(extra_mounts) for container_path, volume_name in ( (LARK_CLI_CONFIG_CONTAINER_PATH, LARK_BROKER_CONFIG_VOLUME_NAME), + (LARK_CLI_LOCKS_CONTAINER_PATH, LARK_BROKER_LOCKS_VOLUME_NAME), (LARK_CLI_DATA_CONTAINER_PATH, LARK_BROKER_DATA_VOLUME_NAME), ): mount = credential_mounts.get(container_path) @@ -757,9 +769,10 @@ def _build_lark_cli_broker_sidecars( ) -> list[k8s_client.V1Container]: """Broker sidecar that holds lark-cli + the per-user credentials (Pattern B). - The config/data credential dirs are mounted **only** here (never on the - sandbox container), so the plaintext app secret / OAuth tokens stay out of - the sandbox filesystem. The broker serves the command surface on loopback. + The config/locks/data dirs are mounted **only** here (never on the sandbox + container), so the plaintext app secret / OAuth tokens stay out of the + sandbox filesystem. The config root is read-only and its nested locks mount + is writable. The broker serves the command surface on loopback. """ if not _lark_cli_broker_enabled(provision_lark_cli_broker): return [] @@ -767,6 +780,7 @@ def _build_lark_cli_broker_sidecars( volume_mounts: list[k8s_client.V1VolumeMount] = [] for container_path, volume_name, sidecar_path in ( (LARK_CLI_CONFIG_CONTAINER_PATH, LARK_BROKER_CONFIG_VOLUME_NAME, LARK_BROKER_SIDECAR_CONFIG_PATH), + (LARK_CLI_LOCKS_CONTAINER_PATH, LARK_BROKER_LOCKS_VOLUME_NAME, LARK_BROKER_SIDECAR_LOCKS_PATH), (LARK_CLI_DATA_CONTAINER_PATH, LARK_BROKER_DATA_VOLUME_NAME, LARK_BROKER_SIDECAR_DATA_PATH), ): mount = credential_mounts.get(container_path)