Commit Graph

8 Commits

Author SHA1 Message Date
jakevin 91c208c855 fix: address deep review findings (security, correctness, consistency) (#935)
* fix: address deep review findings (security, correctness, consistency)

1. Security: add path traversal guard for plugin manifest entry.path
2. Security: sanitize evaluate() index param via JSON.stringify
3. Correctness: fix startNetworkCapture idempotency (don't wipe entries on re-call)
4. Correctness: log pre-navigation failures instead of silently swallowing
5. Consistency: replace console.log/error with log module in commanderAdapter, external
6. Consistency: add PluginError class, convert user-facing plugin errors
7. Dedup: remove local isRecord() in plugin.ts, use shared utils.ts version
8. Clarify: document intentional double validateArgs call

* chore: remove unused chalk imports from external.ts and commanderAdapter.ts

* refactor: replace chalk with Node.js built-in util.styleText

- Remove chalk dependency, use `styleText` from `node:util` (stable in Node 21+)
- Bump engines to Node >= 21
- Update all 10 source files that used chalk
- Remove stale chalk mock from daemon.test.ts
- One fewer runtime dependency

* fix: tighten deep-review follow-up
2026-04-10 22:59:41 +08:00
jakevin ab0af2de5c feat(exit-codes): Unix-standard process exit codes for all error types (#564)
* feat(exit-codes): add Unix-standard exit codes to all CliError types

Introduce EXIT_CODES constant table (sysexits.h conventions) and wire
exitCode into every CliError subclass so the process exit code reflects
the semantic type of failure:

  0   success (default)
  1   generic / unexpected error
  2   argument / usage error        (ArgumentError)
 66   empty result / not found      (EmptyResultError, SelectorError)
 69   service unavailable           (BrowserConnectError, AdapterLoadError)
 77   permission / auth required    (AuthRequiredError)
 78   configuration error           (ConfigError)
124   timeout                       (TimeoutError)
130   Ctrl-C / SIGINT               (unchanged, tui.ts)

resolveExitCode() in commanderAdapter.ts reads err.exitCode for typed
CliErrors, and falls back to pattern-matching message text for untyped
adapter errors (auth pattern → 77, not-found pattern → 66, else → 1).

Shell scripts can now distinguish error categories:
  opencli spotify status || echo "exit $?"   # 69 if browser not running
  opencli github issues --repo x 2>/dev/null; [ $? -eq 77 ] && opencli github auth

* fix(exit-codes): address review findings

- TIMEOUT: change from 124 → 75 (EX_TEMPFAIL); 124 is bash timeout(1)'s
  own exit code, creating ambiguity when shell runs `timeout 30 opencli`
- SelectorError: change from EMPTY_RESULT(66) → GENERIC_ERROR(1); a
  missing DOM selector is an adapter bug, not a user "no data" condition
- normalizeArgValue: throw ArgumentError instead of bare CliError so
  invalid bool args correctly exit with USAGE_ERROR(2) not GENERIC_ERROR(1)
- resolveExitCode: explicitly map 'http' classification to GENERIC_ERROR
  to keep exit-code path in sync with the render path
- tui.ts: replace hardcoded process.exit(130) with EXIT_CODES.INTERRUPTED

* feat(exit-codes): replace all hardcoded exit numbers with EXIT_CODES constants

Extend the exit code system to cover every process exit point in the codebase.
No magic numbers remain — all exit codes are now referenced by name.

Semantic upgrades beyond pure renaming:
- plugin update missing args  → USAGE_ERROR (2) instead of 1
- plugin update conflicting   → USAGE_ERROR (2) instead of 1
- opencli install <unknown>   → USAGE_ERROR (2) instead of 1
- unknown command fallback    → USAGE_ERROR (2) instead of 1
- record with no candidates   → EMPTY_RESULT (66) instead of 1
- external CLI install fail   → SERVICE_UNAVAIL (69) instead of 1
- daemon EADDRINUSE           → SERVICE_UNAVAIL (69) instead of 1

Files touched: cli.ts, external.ts, daemon.ts, main.ts,
               clis/antigravity/serve.ts
2026-03-28 22:16:42 +08:00
jakevin 31cb2291c5 perf: parallel file discovery, plugin scanning, and external CLI caching (#501)
- Parallelize file scanning in discoverClisFromFs and discoverPluginDir
  using Promise.all(files.map(async ...)) instead of serial for-of with
  await, so isCliModule checks run concurrently
- Parallelize plugin directory scanning in discoverPlugins
- Cache loadExternalClis() result to avoid re-parsing YAML on every call
- Invalidate cache in registerExternalCli after writing to disk
- Cache strategyLabel() call in list command to avoid redundant computation
- Add comment explaining why discovery must remain sequential (plugin override semantics)
2026-03-27 14:19:55 +08:00
jakevin 594ad50949 fix: pre-release cleanup — bugs, version sync, and error handling (#412)
* fix: pre-release cleanup — bugs, version sync, and error handling

Bug fixes:
- Fix hasLimit using wrong Set (SEARCH_PARAMS → LIMIT_PARAMS) in
  analysis.ts classifyQueryParams
- Remove phantom scroll step from BROWSER_STEPS and KNOWN_STEP_NAMES
  (declared but never registered, causes runtime crash if used in YAML)
- Add missing download step to KNOWN_STEP_NAMES (was producing
  false-positive validation warnings)

Docs:
- Sync version numbers: SKILL.md, extension/package.json,
  extension/manifest.json → 1.3.3
- Add jd, web to README command tables (both EN and zh-CN)
- Update xueqiu commands with fund-holdings, fund-snapshot

Code quality:
- Replace all 22 catch (err: any) with typed error handling using
  existing getErrorMessage() utility across 13 files

* fix: remove (err as any) casts in error handling

- antigravity/serve.ts: use typed Error.cause instead of (err as any).cause
- external.ts: move instanceof guard into shouldRetryWithCmdShim,
  accept unknown instead of forcing NodeJS.ErrnoException cast at call site
2026-03-25 14:32:29 +08:00
jakevin 53699eb807 fix: harden security-sensitive execution paths (#335)
* fix(security): harden against command injection and sandbox escape

1. cli.ts: Remove auto-discover of arbitrary system binaries via denylist.
   Unknown commands now require explicit registration via `opencli register`.
   The previous denylist approach was trivially bypassable (bash, curl, etc.).

2. template.ts: Protect evalJsExpr against prototype chain escape.
   Block expressions containing constructor/prototype/__proto__/process/etc.
   Deep-copy context objects to sever prototype chains before passing to
   new Function().

3. external.ts: Expand shell operator detection in parseCommand to cover
   $(), $, #, \n, \r — preventing command substitution and comment injection.

4. fetch.ts: Use JSON.stringify for HTTP method in browser evaluate() instead
   of raw string interpolation, preventing JS injection via crafted method values.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: harden security-sensitive execution paths

* chore: tighten template sandbox guard

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-24 11:28:22 +08:00
sline 41aedf68cd fix(external): replace execSync with execFileSync to prevent command injection (#309)
* fix(external): replace execSync with execFileSync to prevent command injection

* fix(review): preserve Windows external installs and restore docs build

* fix(review): preserve Windows external installs after rebase

---------

Co-authored-by: jackwener <jakevingoo@gmail.com>
2026-03-24 00:40:17 +08:00
jakevin eeace115cb fix: harden external CLI hub — command injection, denylist, sync API, build-copy (#149) 2026-03-21 01:26:02 +08:00
jackwener b3e32d8a05 feat: add external CLI hub for discovery, auto-installation, and execution of external tools. 2026-03-20 20:30:40 +08:00