diff --git a/.gitignore b/.gitignore index 8c199271e0..8910d00953 100644 --- a/.gitignore +++ b/.gitignore @@ -332,6 +332,7 @@ assets/gen_typing_gif.py !/docs/PREVIEW_PROGRESS.md !/docs/FREECAD_VIDEO_REFERENCE.md !/docs/PREVIEW_MECHANISM_PROGRESS.md +!/docs/FORK_MAINTENANCE.md !/docs/scripts/ !/docs/scripts/** /docs/scripts/__pycache__/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a485a8962b..df663caf25 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -145,6 +145,12 @@ python3 -m pytest cli_anything//tests/ -v - Click 8.0+ - pytest 7.0+ +### Downstream fork maintenance + +If you maintain a downstream mirror or temporary patch branch, follow +[Downstream Fork Maintenance](docs/FORK_MAINTENANCE.md) before carrying local +changes for more than one upstream sync cycle. + ## Code Style - Follow PEP 8 conventions. diff --git a/cli-anything-plugin/commands/list.md b/cli-anything-plugin/commands/list.md index 36057df9ca..4a8705e1d4 100644 --- a/cli-anything-plugin/commands/list.md +++ b/cli-anything-plugin/commands/list.md @@ -64,7 +64,7 @@ def extract_version_from_setup(setup_path): content = Path(setup_path).read_text() match = re.search(r'version\s*=\s*["\']([^"\']+)["\']', content) return match.group(1) if match else None - except: + except (OSError, UnicodeDecodeError): return None def build_glob_patterns(base_path, depth): diff --git a/cli-hub/README.md b/cli-hub/README.md index 933edcc9c3..c3f0d3fea2 100644 --- a/cli-hub/README.md +++ b/cli-hub/README.md @@ -37,6 +37,25 @@ cli-hub update gimp cli-hub uninstall gimp ``` +## Shell install commands + +Most registry install commands run without a shell. Commands that intentionally +need shell syntax such as pipes, `&&` chains, command substitution, or +redirection must be reviewed in the registry and marked with +`requires_shell: true`. + +If an install fails with `Command contains shell operators and was not +executed`, the registry entry is missing that reviewed opt-in. Use one of these +paths: + +- Prefer updating the registry entry to a safer command shape or adding + `requires_shell: true` after reviewing the command. +- For a one-off local override, run `CLI_HUB_ALLOW_SHELL_COMMANDS=1 cli-hub + install ` only after reviewing the registry entry and install command. + +This applies to script-style entries such as `curl ... | bash` and older +multi-step commands like `cd ... && npm install && npm link`. + ## Preview Viewer `cli-hub` also includes the generic preview consumer for preview-capable diff --git a/cli-hub/cli_hub/installer.py b/cli-hub/cli_hub/installer.py index 0a7451c489..1d3dee779d 100644 --- a/cli-hub/cli_hub/installer.py +++ b/cli-hub/cli_hub/installer.py @@ -1,6 +1,7 @@ """Install, uninstall, and manage CLIs — dispatches to pip or npm based on source.""" import json +import os import shlex import shutil import subprocess @@ -46,24 +47,81 @@ def _find_uv(): ) -_SHELL_METACHARACTERS = ("|", "&&", "||", ";", "$(", "`") - - -def _run_command(cmd): +_ALLOW_SHELL_ENV = "CLI_HUB_ALLOW_SHELL_COMMANDS" + + +def _contains_shell_operator(cmd): + """Return True when cmd contains shell syntax outside literal quoting.""" + quote = None + escaped = False + i = 0 + while i < len(cmd): + ch = cmd[i] + if escaped: + escaped = False + i += 1 + continue + if ch == "\\": + escaped = True + i += 1 + continue + if quote: + if ch == quote: + quote = None + elif quote == '"' and (ch == "`" or (ch == "$" and i + 1 < len(cmd) and cmd[i + 1] == "(")): + return True + i += 1 + continue + if ch in {"'", '"'}: + quote = ch + elif ch in {"|", "&", ";", "<", ">"} or ch in {"\n", "\r"}: + return True + elif ch == "`" or (ch == "$" and i + 1 < len(cmd) and cmd[i + 1] == "("): + return True + i += 1 + return False + + +def _allows_shell(cli): + return bool(cli and cli.get("requires_shell") is True) + + +def _run_command(cmd, *, allow_shell=False): """Run a command string. - Uses shell=True when the command contains shell operators (pipes, &&, etc.) - so that script-type installs like ``curl … | bash`` work correctly. - Commands come from the trusted registry, not from user input. + Simple commands run without a shell. Commands containing shell operators + such as pipes, redirections, or command substitutions require a reviewed + registry entry with ``requires_shell: true``. CLI_HUB_ALLOW_SHELL_COMMANDS=1 + remains a hard override for callers that have reviewed the command locally. """ - use_shell = any(c in cmd for c in _SHELL_METACHARACTERS) + use_shell = _contains_shell_operator(cmd) + if use_shell and not allow_shell and os.environ.get(_ALLOW_SHELL_ENV) != "1": + return subprocess.CompletedProcess( + args=cmd, + returncode=2, + stdout="", + stderr=( + "Command contains shell operators and was not executed. " + "Registry entries that intentionally need shell syntax must " + f"set requires_shell=true. Set {_ALLOW_SHELL_ENV}=1 only after " + "reviewing the registry entry and install command." + ), + ) try: + argv = cmd if use_shell else shlex.split(cmd) return subprocess.run( - cmd if use_shell else shlex.split(cmd), + argv, capture_output=True, text=True, shell=use_shell, ) + except ValueError as exc: + return subprocess.CompletedProcess( + args=cmd, + returncode=2, + stdout="", + stderr=f"Invalid command syntax: {exc}", + ) except FileNotFoundError as exc: missing = exc.filename or shlex.split(cmd)[0] return subprocess.CompletedProcess( @@ -105,7 +163,7 @@ def _generic_install(cli): install_cmd = cli.get("install_cmd") if not install_cmd: return False, f"No install command is defined for {cli['display_name']}." - result = _run_command(install_cmd) + result = _run_command(install_cmd, allow_shell=_allows_shell(cli)) if result.returncode == 0: return True, f"Installed {cli['display_name']} ({cli['entry_point']})" return False, f"Install failed:\n{result.stderr or result.stdout}" @@ -116,7 +174,7 @@ def _generic_uninstall(cli): if not uninstall_cmd: note = cli.get("uninstall_notes") or f"No uninstall command is defined for {cli['display_name']}." return False, note - result = _run_command(uninstall_cmd) + result = _run_command(uninstall_cmd, allow_shell=_allows_shell(cli)) if result.returncode == 0: return True, f"Uninstalled {cli['display_name']}" return False, f"Uninstall failed:\n{result.stderr or result.stdout}" @@ -127,7 +185,7 @@ def _generic_update(cli): if not update_cmd: note = cli.get("update_notes") or f"No update command is defined for {cli['display_name']}." return False, note - result = _run_command(update_cmd) + result = _run_command(update_cmd, allow_shell=_allows_shell(cli)) if result.returncode == 0: return True, f"Updated {cli['display_name']}" return False, f"Update failed:\n{result.stderr or result.stdout}" @@ -203,7 +261,7 @@ def _pip_update(cli): def _uv_install(cli): if _find_uv() is None: return False, _UV_INSTALL_HINT - result = _run_command(cli["install_cmd"]) + result = _run_command(cli["install_cmd"], allow_shell=_allows_shell(cli)) if result.returncode == 0: return True, f"Installed {cli['display_name']} ({cli['entry_point']})" return False, f"uv install failed:\n{result.stderr or result.stdout}" @@ -215,7 +273,7 @@ def _uv_uninstall(cli): uninstall_cmd = cli.get("uninstall_cmd") if not uninstall_cmd: return False, f"No uninstall command is defined for {cli['display_name']}." - result = _run_command(uninstall_cmd) + result = _run_command(uninstall_cmd, allow_shell=_allows_shell(cli)) if result.returncode == 0: return True, f"Uninstalled {cli['display_name']}" return False, f"uv uninstall failed:\n{result.stderr or result.stdout}" @@ -227,7 +285,7 @@ def _uv_update(cli): update_cmd = cli.get("update_cmd") if not update_cmd: return False, f"No update command is defined for {cli['display_name']}." - result = _run_command(update_cmd) + result = _run_command(update_cmd, allow_shell=_allows_shell(cli)) if result.returncode == 0: return True, f"Updated {cli['display_name']}" return False, f"uv update failed:\n{result.stderr or result.stdout}" @@ -312,6 +370,8 @@ def _installed_entry(cli, source, strategy): entry["uninstall_cmd"] = cli["uninstall_cmd"] if cli.get("update_cmd"): entry["update_cmd"] = cli["update_cmd"] + if cli.get("requires_shell") is True: + entry["requires_shell"] = True return entry diff --git a/cli-hub/cli_hub/preview.py b/cli-hub/cli_hub/preview.py index 74174eb477..91b1c1ef0e 100644 --- a/cli-hub/cli_hub/preview.py +++ b/cli-hub/cli_hub/preview.py @@ -116,6 +116,18 @@ def _stringify_command(value: Any) -> Optional[str]: return str(value) +def _script_json(value: Any) -> str: + """Serialize JSON safely for embedding inside a script tag.""" + return ( + json.dumps(value, ensure_ascii=False) + .replace("&", "\\u0026") + .replace("<", "\\u003c") + .replace(">", "\\u003e") + .replace("\u2028", "\\u2028") + .replace("\u2029", "\\u2029") + ) + + def _normalize_index(value: Any, fallback: int) -> int: if isinstance(value, bool): return fallback @@ -625,15 +637,35 @@ def render_session_text(session_ref: str) -> str: return "\n".join(lines) + "\n" -def _artifact_href(output_dir: Path, bundle_dir: Path, artifact_path: str) -> str: - target = (bundle_dir / artifact_path).resolve() +def _resolve_bundle_artifact(bundle_dir: Path, artifact_path: str) -> Optional[Path]: + if not isinstance(artifact_path, str) or not artifact_path.strip(): + return None + if "\x00" in artifact_path: + return None + raw_path = Path(artifact_path) + if raw_path.is_absolute(): + return None + bundle_root = bundle_dir.resolve() + target = (bundle_root / raw_path).resolve() + try: + target.relative_to(bundle_root) + except ValueError: + return None + return target + + +def _artifact_href(output_dir: Path, bundle_dir: Path, artifact_path: str) -> Optional[str]: + target = _resolve_bundle_artifact(bundle_dir, artifact_path) + if target is None: + return None return os.path.relpath(target, output_dir) def _render_artifact_card(output_dir: Path, bundle_dir: Path, artifact: Dict[str, Any]) -> str: role = html.escape(artifact.get("role", "artifact")) label = html.escape(artifact.get("label", artifact.get("artifact_id", "artifact"))) - path_ref = _artifact_href(output_dir, bundle_dir, artifact.get("path", "")) + artifact_path = str(artifact.get("path", "")) + path_ref = _artifact_href(output_dir, bundle_dir, artifact_path) media_type = artifact.get("media_type", "") size = artifact.get("bytes") meta = [] @@ -645,7 +677,13 @@ def _render_artifact_card(output_dir: Path, bundle_dir: Path, artifact: Dict[str meta.append(format_bytes(int(size))) meta_line = " · ".join(meta) - if media_type.startswith("image/"): + if path_ref is None: + body = ( + '
' + f"Unavailable artifact path: {html.escape(artifact_path)}" + "
" + ) + elif media_type.startswith("image/"): body = f'{label}' elif media_type.startswith("video/"): body = ( @@ -715,12 +753,13 @@ def _render_trajectory_html_section(trajectory: Optional[Dict[str, Any]]) -> str ) for item in items ) + empty_timeline_html = '
No step timeline entries yet.
' return ( '
' "

Trajectory

" f'
{cards_html}
' - f'
{items_html or "
No step timeline entries yet.
"}
' + f'
{items_html or empty_timeline_html}
' "
" ) @@ -1363,8 +1402,8 @@ def render_live_html(session_ref: str, output_path: str, poll_ms: int = 1500) -> " + session_path.write_text(json.dumps(session)) + + output_path = tmp_path / "live.html" + render_live_html(str(session_dir), str(output_path), poll_ms=800) + content = output_path.read_text() + + assert " out") + assert result.returncode == 2 + assert "requires_shell=true" in result.stderr + mock_run.assert_not_called() + @patch("cli_hub.installer.subprocess.run") def test_run_command_uses_shell_true_for_and_operator(self, mock_run): - """&& operator also triggers shell=True.""" + """&& operator uses shell=True only after per-entry shell trust.""" mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") - _run_command("curl -O https://example.com/install.sh && bash install.sh") + _run_command("curl -O https://example.com/install.sh && bash install.sh", allow_shell=True) _, kwargs = mock_run.call_args assert kwargs.get("shell") is True @@ -754,9 +853,21 @@ def test_run_command_uses_shell_true_for_and_operator(self, mock_run): @patch("cli_hub.installer.subprocess.run") @patch("cli_hub.installer.get_cli") - @patch("cli_hub.installer.INSTALLED_FILE", Path(tempfile.mktemp())) - def test_install_jimeng_success(self, mock_get_cli, mock_run): - """install_cli('jimeng') succeeds and invokes the pipe command via shell.""" + def test_install_shell_command_blocked_without_requires_shell(self, mock_get_cli, mock_run): + """Registry shell commands must declare per-entry shell trust.""" + mock_get_cli.return_value = {k: v for k, v in JIMENG_CLI.items() if k != "requires_shell"} + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") + + success, msg = install_cli("jimeng") + + assert not success + assert "not executed" in msg + mock_run.assert_not_called() + + @patch("cli_hub.installer.subprocess.run") + @patch("cli_hub.installer.get_cli") + def test_install_jimeng_success_with_requires_shell(self, mock_get_cli, mock_run): + """install_cli('jimeng') can run because the reviewed entry opted in.""" mock_get_cli.return_value = JIMENG_CLI mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") @@ -772,7 +883,22 @@ def test_install_jimeng_success(self, mock_get_cli, mock_run): @patch("cli_hub.installer.subprocess.run") @patch("cli_hub.installer.get_cli") - @patch("cli_hub.installer.INSTALLED_FILE", Path(tempfile.mktemp())) + def test_install_sketch_uses_command_without_shell(self, mock_get_cli, mock_run): + """install_cli('sketch') uses the reviewed npm command without shell=True.""" + mock_get_cli.return_value = SKETCH_CLI + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") + + success, msg = install_cli("sketch") + + assert success, f"Expected success but got: {msg}" + assert "Sketch" in msg + args = mock_run.call_args[0][0] + _, kwargs = mock_run.call_args + assert args == ["npm", "install", "-g", "./sketch/agent-harness"] + assert kwargs.get("shell") is False or kwargs.get("shell") is None + + @patch("cli_hub.installer.subprocess.run") + @patch("cli_hub.installer.get_cli") def test_install_jimeng_failure_propagated(self, mock_get_cli, mock_run): """A non-zero exit from the curl|bash script surfaces as failure.""" mock_get_cli.return_value = JIMENG_CLI @@ -798,10 +924,9 @@ def test_uninstall_jimeng_no_cmd_returns_graceful_message(self, mock_get_cli): @patch("cli_hub.installer.subprocess.run") @patch("cli_hub.installer.get_cli") - @patch("cli_hub.installer.INSTALLED_FILE", Path(tempfile.mktemp())) - def test_install_jimeng_recorded_in_installed_json(self, mock_get_cli, mock_run): + def test_install_jimeng_recorded_in_installed_json(self, mock_get_cli, mock_run, tmp_path): """After a successful install, jimeng appears in installed.json.""" - installed_file = Path(tempfile.mktemp()) + installed_file = tmp_path / "jimeng-installed.json" mock_get_cli.return_value = JIMENG_CLI mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") @@ -812,6 +937,7 @@ def test_install_jimeng_recorded_in_installed_json(self, mock_get_cli, mock_run) assert "jimeng" in data assert data["jimeng"]["strategy"] == "command" assert data["jimeng"]["package_manager"] == "script" + assert data["jimeng"]["requires_shell"] is True # ─── Analytics tests ────────────────────────────────────────────────── diff --git a/docs/FORK_MAINTENANCE.md b/docs/FORK_MAINTENANCE.md new file mode 100644 index 0000000000..6b86167ae2 --- /dev/null +++ b/docs/FORK_MAINTENANCE.md @@ -0,0 +1,75 @@ +# Downstream Fork Maintenance + +This repository is maintained as a close mirror of `HKUDS/CLI-Anything`. +Downstream-only changes should stay small, temporary, and upstreamable. + +## Policy + +- Treat `upstream/main` as the source of truth. +- Use local branches only for focused fixes or validation work. +- Do not accumulate long-lived downstream feature patches. +- Prefer upstream pull requests for bug fixes, compatibility fixes, tests, and documentation. +- Keep local-only operational notes outside the upstreamable patch when they are not generally useful. + +## Sync Routine + +```bash +git fetch upstream --prune +git switch main +git merge --ff-only upstream/main +git switch -c fix/ +``` + +If local work already exists, rebase it onto the fetched upstream head: + +```bash +git fetch upstream --prune +git rebase upstream/main +``` + +When rebase conflicts are broad or unclear, stop and re-check whether the local patch is still needed before resolving by hand. + +## Patch Workflow + +1. Reproduce the issue on the current upstream baseline. +2. Make the smallest patch that fixes the upstream-visible behavior. +3. Add or update tests when the repository has a nearby test surface. +4. Run the narrowest relevant checks first. +5. Run the broader checks that protect the touched surface. +6. Open an upstream pull request against `HKUDS/CLI-Anything:main`. +7. Keep the downstream branch only until upstream accepts the fix or makes it obsolete. + +## Validation Checklist + +For Python compatibility fixes: + +```bash +python3 -m py_compile +python3.10 -m py_compile # when available +python3.11 -m py_compile # when available +``` + +For command-doc or plugin-command fixes: + +```bash +rg '^\s*except\s*:' opencode-commands cli-anything-plugin cli-hub +``` + +For harness changes: + +```bash +python3 -m pytest /agent-harness/cli_anything//tests/test_core.py -q +``` + +Run `git diff --check` before publishing a branch. + +## Upstream PR Notes + +The pull request should state: + +- the upstream-visible bug or compatibility issue, +- the Python versions or harnesses tested, +- the exact validation commands used, +- whether the change affects only docs, generated command files, a harness, or shared runtime code. + +Avoid mixing unrelated harness changes in the same pull request. diff --git a/opencode-commands/cli-anything-list.md b/opencode-commands/cli-anything-list.md index cfd31a5cc4..929e60eb8b 100644 --- a/opencode-commands/cli-anything-list.md +++ b/opencode-commands/cli-anything-list.md @@ -63,7 +63,7 @@ def extract_version_from_setup(setup_path): content = Path(setup_path).read_text() match = re.search(r'version\s*=\s*["\']([^"\']+)["\']', content) return match.group(1) if match else None - except: + except (OSError, UnicodeDecodeError): return None def build_glob_patterns(base_path, depth): diff --git a/public_registry.json b/public_registry.json index 173c37ac09..b802a32982 100644 --- a/public_registry.json +++ b/public_registry.json @@ -286,6 +286,7 @@ "source_url": null, "package_manager": "script", "install_strategy": "command", + "requires_shell": true, "install_cmd": "curl -s https://jimeng.jianying.com/cli | bash", "skill_md": "https://bytedance.larkoffice.com/wiki/FVTwwm0bGiishxkKOoScdHR2nsg", "entry_point": "dreamina", diff --git a/registry.json b/registry.json index ffa3928b7d..5e10f9a63e 100644 --- a/registry.json +++ b/registry.json @@ -757,7 +757,9 @@ "requires": "Node.js >= 16.0.0", "homepage": "https://www.sketch.com", "source_url": null, - "install_cmd": "cd sketch/agent-harness && npm install && npm link", + "package_manager": "npm", + "install_strategy": "command", + "install_cmd": "npm install -g ./sketch/agent-harness", "entry_point": "sketch-cli", "skill_md": null, "category": "design", diff --git a/unimol_tools/agent-harness/cli_anything/unimol_tools/tests/test_full_e2e.py b/unimol_tools/agent-harness/cli_anything/unimol_tools/tests/test_full_e2e.py index 345f43b941..9c39d7f41b 100644 --- a/unimol_tools/agent-harness/cli_anything/unimol_tools/tests/test_full_e2e.py +++ b/unimol_tools/agent-harness/cli_anything/unimol_tools/tests/test_full_e2e.py @@ -63,7 +63,7 @@ def test_project(): # Cleanup: Delete project (if implemented) try: run_cli_command(["project", "delete", "--name", project_name]) - except: + except Exception: pass diff --git a/unimol_tools/agent-harness/cli_anything/unimol_tools/utils/weights.py b/unimol_tools/agent-harness/cli_anything/unimol_tools/utils/weights.py index ccb6f5bba3..57c81e22ba 100644 --- a/unimol_tools/agent-harness/cli_anything/unimol_tools/utils/weights.py +++ b/unimol_tools/agent-harness/cli_anything/unimol_tools/utils/weights.py @@ -117,7 +117,7 @@ def get_weight_info(): "custom_dir": 'UNIMOL_WEIGHT_DIR' in os.environ, "exists": os.path.exists(weighthub.WEIGHT_DIR) } - except: + except (ImportError, AttributeError, OSError): return { "error": "unimol_tools not available" }