diff --git a/README.md b/README.md index 1512df2aa6..ac72b24ac2 100644 --- a/README.md +++ b/README.md @@ -1324,7 +1324,7 @@ Skill installs and agent-managed skill edits run through **SkillScan**, a native Windows scripts (`.bat`, `.cmd`, `.ps1`, `.psm1`, `.js`, `.jse`, `.vbs`, `.vbe`, `.wsf`), HTML applications (`.hta`), and scriptlets (`.sct`) count as code even outside `scripts/`, regardless of filename case. They receive both SkillScan analysis and the installer's executable-code policy. SkillScan warns about remote downloads piped into common shells, including sudo, interpreter paths, and shell line continuations. Pipes to non-shell tools such as `jq` and `tee` do not trigger this warning. -SkillScan treats HTTP hostnames case-insensitively and recognizes bracketed IPv6 +SkillScan treats HTTP/HTTPS scheme spellings and HTTP hostnames case-insensitively and recognizes bracketed IPv6 loopback (`[::1]`) URLs as local. External IPv6 endpoints still trigger network findings, and cloud-metadata hostname detection is case-insensitive. diff --git a/backend/packages/harness/deerflow/skills/AGENTS.md b/backend/packages/harness/deerflow/skills/AGENTS.md index ea2244f060..30370f4afc 100644 --- a/backend/packages/harness/deerflow/skills/AGENTS.md +++ b/backend/packages/harness/deerflow/skills/AGENTS.md @@ -17,7 +17,7 @@ - **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, 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 paths, matched in order through interleaved option values, 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`), plus `sandbox_runtime_probed` separating a confirmed probe result from the conservative unprobed fallback (mutation responses carry a probed status built from a credential snapshot taken under the credential lock), 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()`. The moderation adapter must normalize both plain-text responses and LangChain Responses API text blocks before parsing the required JSON decision. `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. `skills/package_files.py` is the single definition of code files (`scripts/` members, code suffixes, extensionless `#!` files) and executable magic bytes; the installer, export guard, and SkillScan all import it, so do not re-derive either rule locally. Files that fail NUL-free UTF-8 decoding are treated as binaries unless they are code files: interpreters still run those, so they raise `package-undecodable-script` (HIGH) and are analyzed over a lossy decode, keeping one stray byte from hiding a file or downgrading a `CRITICAL` match; one with executable magic skips the text rules, which only misread its string tables. 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`. -- **SkillScan HTTP hosts**: use URL authority parsing for lowercase hosts and bracketed IPv6 loopback; userinfo must never hide an external host. Malformed URLs remain outbound for exfiltration checks. Cleartext HTTP detection covers IPv6 literals; cloud-metadata hostname matching is case-insensitive. Coverage: `tests/test_skillscan_native.py`. +- **SkillScan HTTP hosts**: recognize HTTP/HTTPS schemes case-insensitively in both URL regexes and prefix checks, without changing source evidence. Use URL authority parsing for lowercase hosts and bracketed IPv6 loopback; userinfo must never hide an external host. Malformed URLs remain outbound for exfiltration checks. Cleartext HTTP detection covers IPv6 literals; cloud-metadata hostname matching is case-insensitive. Coverage: `tests/test_skillscan_native.py`. - **Python secret mappings**: `secret-env-assignment` scans literal container values and conditional branches under secret-like bindings. Dictionary keys are structural labels and contribute candidates only when their folded literal matches the shared `_SECRET_TOKEN_PATTERNS`; they must not inherit credential evidence from the enclosing variable name. Opaque keys without a recognized token format stay unreported. Regressions in `tests/test_skillscan_native.py` cover runtime-loaded mappings, nested labels, credential-value locations, and literal/concatenated token keys; `tests/test_skill_review_core.py` pins the CI CLI gate. - **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 analyzer hands SkillScan a byte-faithful copy of every non-symlink snapshot file (binary entries carry `content_base64`), so package rules such as `package-executable-binary` cover binaries; only eval-fixture `SKILL.md` samples are withheld, while scripts and other files under `evals/fixtures/` are scanned. Materialization uses exclusive create, so a duplicate archive member or a case-folded name fails the scan closed (`not_assessed: skillscan`) instead of overwriting an earlier file, as does an entry without bytes in an untruncated snapshot. 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/skills/skillscan/orchestrator.py b/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py index 4ac1d9f822..4dfde9c01d 100644 --- a/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py +++ b/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py @@ -135,8 +135,8 @@ _SECRET_TOKEN_PATTERNS = tuple( ) ) _SENSITIVE_PATH_RE = re.compile(r"(~/.ssh|/etc/passwd|/etc/shadow|/var/run/docker\.sock|docker\.sock|169\.254\.169\.254)") -_EXTERNAL_HTTP_RE = re.compile(r"http://(?:[^/?#\s)'\"<>]*@)?(\[[0-9A-Fa-f:.]+\]|[A-Za-z0-9.-]+)(?::\d+)?(?:/|\b|(?=$|[\s)'\"<>?#]))") -_URL_RE = re.compile(r"https?://[^\s)'\"<>]+") +_EXTERNAL_HTTP_RE = re.compile(r"(?i:http)://(?:[^/?#\s)'\"<>]*@)?(\[[0-9A-Fa-f:.]+\]|[A-Za-z0-9.-]+)(?::\d+)?(?:/|\b|(?=$|[\s)'\"<>?#]))") +_URL_RE = re.compile(r"(?i:https?)://[^\s)'\"<>]+") _LOCAL_HTTP_HOSTS = {"localhost", "127.0.0.1", "0.0.0.0", "::1"} # `rm` with a recursive flag (any order/combination, optional --no-preserve-root) # targeting the filesystem root, a wildcard, or a complete system-root directory. @@ -624,7 +624,7 @@ def _scan_declaration(rel_path: str, text: str) -> list[SecurityFinding]: findings.append(_finding_from_match("declaration-sensitive-path", rel_path, text, match)) for match in _URL_RE.finditer(text): - if match.group(0).startswith("http://"): + if match.group(0)[:7].lower() == "http://": host = _http_host(match.group(0)) if host and host not in _LOCAL_HTTP_HOSTS: findings.append(_finding_from_match("declaration-external-endpoint", rel_path, text, match)) @@ -1017,7 +1017,7 @@ def _looks_like_placeholder(value: str) -> bool: def _http_host(url: str) -> str | None: - if not url.startswith(("http://", "https://")): + if not url[:8].lower().startswith(("http://", "https://")): return None try: # Parse the authority so IPv6 brackets and userinfo cannot be mistaken @@ -1029,7 +1029,7 @@ def _http_host(url: str) -> str | None: def _is_outbound_url(value: str) -> bool: - return bool(value.startswith(("http://", "https://")) and (_http_host(value) or "") not in _LOCAL_HTTP_HOSTS) + return bool(value[:8].lower().startswith(("http://", "https://")) and (_http_host(value) or "") not in _LOCAL_HTTP_HOSTS) def _collect_python_aliases(tree: ast.AST) -> dict[str, str]: diff --git a/backend/tests/test_skillscan_native.py b/backend/tests/test_skillscan_native.py index e9c14b2be1..f6a32feb0a 100644 --- a/backend/tests/test_skillscan_native.py +++ b/backend/tests/test_skillscan_native.py @@ -1905,6 +1905,9 @@ def test_shell_curl_without_pipe_finishes_on_repeated_backslash_text(tmp_path: P "url, host", [ ("http://LOCALHOST:8080/api", "localhost"), + ("HTTP://LOCALHOST:8080/api", "localhost"), + ("HtTp://Example.COM/api", "example.com"), + ("HTTPS://[::1]:8443/api", "::1"), ("http://[::1]/api", "::1"), ("https://[::1]:8443/api", "::1"), ("http://[2001:DB8::1]:8080/api", "2001:db8::1"), @@ -1934,9 +1937,10 @@ def test_uppercase_local_host_is_classified_local(tmp_path: Path) -> None: @pytest.mark.parametrize("host, external", [("localhost", False), ("LOCALHOST", False), ("LocalHost", False), ("Example.COM", True), ("[::1]", False), ("[2001:DB8::1]", True), ("localhost@Example.COM", True)]) -def test_declared_http_host_case_classification(tmp_path: Path, host: str, external: bool) -> None: +@pytest.mark.parametrize("scheme", ["http", "HTTP", "HtTp"]) +def test_declared_http_host_case_classification(tmp_path: Path, host: str, external: bool, scheme: str) -> None: skill_dir = tmp_path / "skill" - _write_skill(skill_dir, f"Endpoint: http://{host}:8080/api\n") + _write_skill(skill_dir, f"Endpoint: {scheme}://{host}:8080/api\n") findings = scan_skill_dir(skill_dir)["findings"] @@ -1945,10 +1949,11 @@ def test_declared_http_host_case_classification(tmp_path: Path, host: str, exter @pytest.mark.parametrize("host, external", [("localhost", False), ("LOCALHOST", False), ("LocalHost", False), ("Example.COM", True), ("[::1]", False), ("[2001:DB8::1]", True), ("[::1]@Example.COM", True)]) -def test_sensitive_path_http_host_case_classification(tmp_path: Path, host: str, external: bool) -> None: +@pytest.mark.parametrize("scheme", ["http", "HTTP", "HtTp", "https", "HTTPS", "HtTpS"]) +def test_sensitive_path_http_host_case_classification(tmp_path: Path, host: str, external: bool, scheme: str) -> None: skill_dir = tmp_path / "skill" _write_skill(skill_dir) - (skill_dir / "run.py").write_text(f'ENDPOINT = "http://{host}:8080/api"\nopen("/etc/passwd").read()\n', encoding="utf-8") + (skill_dir / "run.py").write_text(f'ENDPOINT = "{scheme}://{host}:8080/api"\nopen("/etc/passwd").read()\n', encoding="utf-8") result = scan_skill_dir(skill_dir) findings = result["findings"] @@ -1962,18 +1967,19 @@ def test_sensitive_path_http_host_case_classification(tmp_path: Path, host: str, @pytest.mark.parametrize( - "url, local", + "endpoint, local", [ - ("http://[::1]:8080/api", True), - ("http://[::1]", True), - ("http://[::1]?mode=local", True), - ("http://[2001:DB8::1]:8080/api", False), - ("http://[2001:db8::1]", False), + ("[::1]:8080/api", True), + ("[::1]", True), + ("[::1]?mode=local", True), + ("[2001:DB8::1]:8080/api", False), + ("[2001:db8::1]", False), ], ) -def test_ipv6_cleartext_http_classification(tmp_path: Path, url: str, local: bool) -> None: +@pytest.mark.parametrize("scheme", ["http", "HTTP", "HtTp"]) +def test_ipv6_cleartext_http_classification(tmp_path: Path, endpoint: str, local: bool, scheme: str) -> None: skill_dir = tmp_path / "skill" - _write_skill(skill_dir, f"# Demo\nEndpoint: {url}\n") + _write_skill(skill_dir, f"# Demo\nEndpoint: {scheme}://{endpoint}\n") findings = scan_skill_dir(skill_dir)["findings"] @@ -1985,10 +1991,11 @@ def test_ipv6_cleartext_http_classification(tmp_path: Path, url: str, local: boo assert not [item for item in findings if item["rule_id"] == other_rule] -def test_malformed_ipv6_url_remains_outbound(tmp_path: Path) -> None: +@pytest.mark.parametrize("scheme", ["http", "HTTP", "HtTp", "https", "HTTPS"]) +def test_malformed_ipv6_url_remains_outbound(tmp_path: Path, scheme: str) -> None: skill_dir = tmp_path / "skill" _write_skill(skill_dir) - (skill_dir / "run.py").write_text('ENDPOINT = "http://[::1/api"\nopen("/etc/passwd").read()\n', encoding="utf-8") + (skill_dir / "run.py").write_text(f'ENDPOINT = "{scheme}://[::1/api"\nopen("/etc/passwd").read()\n', encoding="utf-8") result = scan_skill_dir(skill_dir) @@ -1997,9 +2004,10 @@ def test_malformed_ipv6_url_remains_outbound(tmp_path: Path) -> None: assert result["scanner_errors"] == [] -def test_cleartext_http_uses_host_after_userinfo_without_exposing_credentials(tmp_path: Path) -> None: +@pytest.mark.parametrize("scheme", ["http", "HTTP", "HtTp"]) +def test_cleartext_http_uses_host_after_userinfo_without_exposing_credentials(tmp_path: Path, scheme: str) -> None: skill_dir = tmp_path / "skill" - _write_skill(skill_dir, "Endpoint: http://localhost:private-value@Example.COM:8080/api\n") + _write_skill(skill_dir, f"Endpoint: {scheme}://localhost:private-value@Example.COM:8080/api\n") findings = scan_skill_dir(skill_dir)["findings"]