main
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 WODE25500
|
0389ace563 |
fix: platform-aware shell quoting + PowerShell call operator (win32) (#246)
dsh's ctx.shell is the platform executor: on win32 the bash stack is disabled and ctx.shell is a PowerShell executor (powershell.exe 5.1 / pwsh 7), on POSIX it is bash. The plugin emitted POSIX quotes unconditionally, so every tool command failed on Windows: 'python' 'args' parses as a string-array expression and errors with 'missing call operator' (bare quoted words are not a command in PowerShell). - q(): platform-aware quoting — PowerShell single-quote escaping (doubled quote for an embedded quote) on win32, POSIX (close/reopen) elsewhere. - quoteArgv(): prepend the `&` call operator on win32 so the quoted argv runs as a command; CRLF/lone CR folded to one space, C0 stripped. - canary/audits: host-shell aware (discover PowerShell 5.1/pwsh 7 like dsh-pwsh-local, or bash); injection + control-char audits run against the real host shell. Verified: canary 40/40 (platform-aware quoting), injection 7/7 + control-char 7/7 under real Windows PowerShell, real dsh 0.1.1-rc.2 skillopt_status runs. POSIX (bash) path unchanged — existing quoting suite still passes. Co-authored-by: WODE25500 <WODE25500@users.noreply.github.com> |
||
|
WODE25500
|
6fc20f33c6 |
Add DeepSeek Harness (dsh) integration to SkillOpt-Sleep plugins (#237)
* Add DeepSeek Harness (dsh) integration
New plugins/dsh/ integration wrapping the shared skillopt_sleep engine
for DeepSeek Harness: a Cordis plugin registering 7 native skillopt_*
tools (status/dry-run/run/adopt/harvest/schedule/unschedule), a bundled
SKILL.md, a bundle patch layer (cordis.patch.yml), and a bootstrap
script. Register the plugin in the plugins/README.md integration table.
* Fix dsh integration per review: safe argv, operator-only auto-adopt, parity tests, English skill
Addresses all review points from the SkillOpt maintainer.
Blocker 1 — shell injection / broken documented example:
- Replace buildCommand() (string join, no quoting) with buildArgv() returning
an argv array; execute() quotes every element with the POSIX-safe '\'' spelling
before shell.resolve(). Model/config-controlled values (project, model,
preferences, source) cannot break out of their argument — verified with a
real-bash injection audit (7 payloads). The documented preferences example
now round-trips as one argument.
- Resolve scripts/sleep.py via an absolute path from the plugin dir so it works
regardless of the dsh cwd.
Blocker 2 — auto-adopt no longer model-callable:
- autoAdopt was a model-facing tool parameter forwarding --auto-adopt. Moved to
operator-only config (default false); the tool parameter is removed. The
canary asserts a model-supplied autoAdopt is ignored.
Should fix — plugin registry parity test:
- Register dsh SKILL.md in tests/test_plugin_sync.py PLUGIN_SKILL_MDS. The
parity tests now cover dsh (backends, schedule/unschedule, memory
consolidation). 13/13 pass.
Minor — English-first skill doc:
- SKILL.md rewritten in English; Chinese README stays as README.zh.md.
Runtime correctness (from the first review round):
- execute() goes through shell.resolve() so workdir/output-cap/sandbox defaults apply.
- Consumes rc.8 CollectedOutput { text, truncated, spillPath }; distinguishes
timeout (exit=timeout) from abort (exit=signal).
- package.json includes cordis.patch.yml in files and declares schemastery.
- scripts/sleep.py mirrors the official runner (repo-root resolution, Python >=
3.10 selection, CLI/installed-package fallback).
- New scripts/canary.mjs: pack + load + invoke checks.
Tested locally: canary 21 checks, real-bash quoting 10 checks, real-DSH (rc.6)
13 checks, repo parity 13/13 — no regressions, nothing touches the shared engine.
* Add LICENSE, portable test scripts; align README.zh.md and pack files with the established plugin pattern
* Security: strip control chars in argv quoting (defense in depth)
Model-controlled values containing \\r, \\r\\n or other control characters
would split a single-quoted word into multiple argv words (broken command,
not RCE — quotes never execute), and corrupt the engine's arg parsing. Strip
C0 control characters to a space so every value arrives as exactly one
argument. Verified: new audit-control-chars.mjs covers \\n, \\r, \\r\\n, tab,
NUL, backtick, quotes — all neutralized (single arg, no file, no execution).
* Fix dsh install command in READMEs: dsh is a global CLI, not a pnpm dependency
The previous form 'pnpm dsh web --patch ...' made pnpm try to fetch a
nonexistent @deepseek-ai/dsh-type-meta package and fail with 404. dsh is
installed as a global CLI; the correct overlay invocation is
'dsh web --patch ./plugins/dsh/cordis.patch.yml' (verified with --dump-config).
* Security: enforce per-tool parameter whitelist (block undeclared arg injection)
dsh's parameter schema accepts undeclared properties by default (no
additionalProperties:false), and buildArgv() forwarded both model-supplied
values and operator config defaults for every known key to the engine. A
model (or prompt-injected transcript) could therefore pass backend/model/
json/editBudget/etc. to tools that do not declare them — including
skillopt_adopt, the live-change boundary.
- buildArgv() now takes an explicit per-tool llowed key set; keys outside
it are neither read from args nor filled from config defaults.
- Each tool's build() passes exactly the keys it declares (whitelist).
- canary.mjs: new 7b step asserts adopt drops undeclared backend/model/
maxTasks/json while keeping declared project; step 4 now drives the
nonzero-exit path via preferences (a declared run parameter).
- audit-*.mjs: BASH_PATH env override for non-Windows portability.
* Security: value-domain guard for path params; unschedule --all is operator-only
The engine re-interpolates model-supplied values into its OWN shell command
strings: scheduler.py splices --project into a crontab line and a Windows
run.cmd executed by schtasks (no escaping), and write_tasks_file() turns an
arbitrary --output into abspath+makedirs+overwrite. argv-level quoting in the
plugin protects the dsh bash -c boundary but cannot protect those secondarysplices. A model-controlled project containing shell metacharacters (quote,
ampersand, semicolon, pipe, dollar, backtick, angle brackets, braces, glob,
control chars) would break out and execute as a separate command under thescheduler shell; an absolute or traversal output would overwrite an arbitrary
file.
- assertSafePath(): rejects shell metacharacters in project and output values.
- assertSafeOutput(): refuses absolute paths and .. traversal for --output.
- execute() runs both guards before buildArgv, so a bad value never reaches
the engine; the rejection is returned to the model as tool output.
- skillopt_unschedule: removed model-callable --all; now operator-only via
config.unscheduleAll (same pattern as autoAdopt).
- canary.mjs: new 7c step asserts injected project / absolute / traversal
output are rejected and legit paths pass (32 checks total).
* Security: clock range guard for schedule; pin dependency versions
- schedule hour/minute were spliced by the engine into a crontab line and a
schtasks start time without validation; out-of-range values (99, -1) would
create broken scheduled entries. execute() now enforces hour in [0,23] and
minute in [0,59] before building argv.
- package.json: replace bare '*' dependency ranges with known-good pinned
versions (@deepseek-ai/schemastery ^3.18.1, cordis ^4.0.1, dsh-tools
^0.1.0-rc.8) so installs are reproducible and not silently broken by a
future upstream release.
- canary.mjs: new 7d step asserts hour=99 / minute=-1 are rejected and legit
clock values pass (35 checks total).
* Align with DSH ecosystem plugin conventions; document both patch-invocation forms
- package.json: add peerDependenciesMeta marking @deepseek-ai/cordis and
@deepseek-ai/dsh-tools optional, matching the official ecosystem practice
(dsh-office-tools et al. declare host-provided peers optional). Without it a
plain 'npm install dsh-skillopt' would hard-fail when the host DSH version
differs from the pinned peer range, instead of warning.
- README.md / plugins/README.md: document BOTH overlay forms - 'pnpm dsh web
--patch' for a DeepSeek Harness source checkout (the official dev workflow)
and 'dsh web --patch' for a globally installed dsh.
* Docs: fix parameter name in SKILL.md (maxTasks, not max_tasks)
The skill's parameter table listed max_tasks (snake_case) but the tools declare
maxTasks (camelCase); a model following the skill doc would send max_tasks and be
rejected by dsh's parameter validation (undeclared property).
* Canary: actually pack + extract and load the packed bundle (review requirement)
The review asked for a clean-package canary that 'loads the packed bundle'.
The previous canary verified the pack file list via --dry-run but then imported
the plugin from the source tree. It now runs 'npm pack --json', extracts the
tarball, and loads src/index.js FROM THE EXTRACTED package/ artifact for every
step (register, status, error paths, quoting, whitelist, value guard, clock),
so the artifact under test is exactly what the 'files' list ships. Tarball and
scratch dir are removed on exit.
* Docs: complete README config keys table (all schema keys, corrected module default)
The config keys table now lists every Config schema key (added engineScript,
scope, autoAdopt, unscheduleAll, timeoutMs) and no longer claims module defaults
to 'skillopt_sleep' (the default path is the scripts/sleep.py bootstrap; module
is an explicit override).
---------
Co-authored-by: WODE25500 <WODE25500@users.noreply.github.com>
|