Skip to content

a task param reaches its command only as shell SOURCE: there is no data channel into a command step process environment #34

Closed

Description

@aitrawog

What

A task's params: are substituted into a plan step's text and nowhere else. There is no channel that carries a param's value into a command step's process environment as data.

Measured on opencharly/opencharly with the worktree-local build (charly version → 2026.295.0418), against a disposable fixture manifest:

$ charly task channels
task channels: 2 step(s), 0 failed
  [pass] check — the runner's own substitution
  [pass] check — the step's own environment

with the two steps' own output:

RUNNER=[${out}] [DEFAULT-UPPER]        # the runner's substitution
(no out/UPPER in the step's process env)  # the step's environment

The step's process env carries neither the param nor the task's merged env.

RCA

Mechanism — three facts, each in the source.

  1. The merged env is the expansion map only. The task runner builds env := workflowkit.MergedEnv(t, params) (plugin-task/candy/plugin-task/runner.go:212) and hands it to the plan runner as kit.RunnerConfig.Env (runner.go:236-241) — the map the expander resolves ${…} against. It is never exported into a step's process environment; the step's process env is the operator's inherited env plus the step's own env: modifier.
  2. The step's env: modifier is the only environment channel, and it is not substitutable. The command verb applies op.Env verbatim as shell exports (plugin-command/candy/plugin-command/plugin.go:78-84), and op.Env is not in Op.StringFields() (spec/spec/charly_methods.go:110-141), so a param cannot be substituted into it either. It is a literal map the author writes by hand.
  3. What is left is text substitution, and the text is parsed by a shell. The command verb runs the step's text through exec.CommandContext(ctx, "sh", "-c", cmd) (plugin-command/candy/plugin-command/plugin.go:112), and the expander rewrites the text before the shell sees it (sdk/kit/checkvars_expand.go:117-148).

Consequence — a value that is data can only be delivered as source. A param's value lands in the command's text, so the shell parses it. Measured, with the value supplied on the command line:

$ charly task source --param 'EVIL=$(touch /tmp/r1-repro/PWNED2)'
task source: 1 step(s), 0 failed
  [pass] check — the value reaches the shell as source
$ ls -la /tmp/r1-repro/PWNED2
-rw-r--r-- 1 atrawog atrawog 0 Oct 22 16:47 /tmp/r1-repro/PWNED2

The marker exists: the shell executed the caller's value. The same holds for a value that merely contains a space, a ;, or a glob — it is not a value, it is shell text.

Second consequence — a param is only reachable if its NAME is uppercase. The expander's grammar is uppercase-initial only (sdk/kit/checkvars_expand.go:16-17: TestVarRefPattern = \$\{([A-Z_][A-Z0-9_]*)(?::([^}]+))?\} — "uppercase-underscore names"). A lowercase param is in the map and unreachable from the plan:

reference in a command's text what the runner does measured
${UPPER} (uppercase param) substitutes the param's value RUNNER=[${out}] [DEFAULT-UPPER] → the UPPER slot reads DEFAULT-UPPER
${out} (lowercase param) does not match the grammar — the reference survives into the shell, where out is not set, so it expands to nothing the out slot reads ${out} (the shell ate it)
${OUT} (uppercase param) substitutes GOT=[/tmp/r1-repro/OUTVALUE]

Missed control. Nothing asserts that a task's declared params are reachable from its own plan. The consequence was found in the wild: opencharly/plugin-marketplace's own skills:drift gate is red for exactly this reason (its param is named out), and that repo has no CI step that runs the task, so the gate has been red without anyone seeing it — filed separately as opencharly/plugin-marketplace#42.

Blast radius. Every task in the org whose plan reads a param, and every caller who passes --param. A value that must be a path, a URL, a name or a JSON document can only be expressed as shell text.

What I would expect

Any one of these ends it:

  • a data channel into the step's process environment — the task's merged env (vars + env + resolved params) exported into a command step's environment, so a param reaches a command as a value the shell never re-parses. This is the channel the env: modifier already is, except that its values would come from the resolved map instead of from authored text;
  • or the expander's grammar widened to cover the task's declared param names, so a lowercase param is substitutable (with the same care: the substituted value would still land in shell text, so this fixes reachability but not the data/source confusion);
  • or a step form that takes an argv (command: [string, ...string], as spec/schema/agent.cue already allows) and is executed without a shell, so a value is a value.

A value that is data must never be parsed by a shell on its way to a command.

Scope

Filed from opencharly/opencharly (wave 1 of the light-clone work, opencharly/opencharly#588), where the same seam made the umbrella's own verify step carry a braced reference. The umbrella's occurrence is fixed in that PR; this issue owns the product capability. The fix may well belong in the sdk/kit expansion seam (sdk/kit/checkvars_expand.go, sdk/kit/planrun.go) rather than in this plugin — this issue is filed here because the task verb owns the task's params, its merged env, and the step env it hands to the runner (plugin-task/candy/plugin-task/runner.go:212-241).

Agent: charly

Assisted-by: Claude Code minimax-m3.1-flash (analysed on a live system)

Activity

  1. aitrawog commented on Oct 24, 2026

    @aitrawog
    ContributorAuthor

    CLAIM — I am taking this issue; it is mine (filed from the umbrella's wave-1 work) and it is unassigned.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

    What I read from the issue. The finding is a channel defect, not a parsing defect: a task's params: are substituted into a plan step's text, and the text is then handed to a shell, so a value that is data can only be delivered as source. The issue measured the two consequences on a live system — the shell executing a caller's value ($(touch …)), and a lowercase param being unreachable because the expander's grammar is uppercase-only.

    What I will do. Fix the channel, not the grammar: give a step a data channel for the task's resolved values, so a param reaches a command as a value the shell never re-parses. The issue's own first "what I would expect" bullet names that shape, and it closes both measured consequences at once — the value arrives as data, and a lowercase param becomes reachable by name because the command reads it from its environment rather than from shell text.

    Where the change lands. I will place it by ownership, not by convenience: the task verb owns the task's params and its merged env, and the step's process environment is applied by the step's own verb. I am reading the runner, the kit expansion seam and the command verb before I fix the seam, and I will state the placement in the PR body with the source citations that decide it — the issue is explicit that the fix may belong in the sdk/kit expansion seam rather than in this plugin, and I will follow the ownership, not the issue's guess about the file.

    Gate. The live proof is the repo's own scripts/gate-task.sh — the only live execution of the five maintenance verbs — plus the module's Go gates under the module's own toolchain, and the coverage must fail against the current tree (a fixture that passes before the change proves nothing).

    Next update. I will post the design and the measured before/after when the fixture exists. If the placement decision turns out to be a genuine crossroad between two repos, I will say so on this thread rather than silently narrowing the scope.

  2. self-assigned this
    on Oct 24, 2026
  3. aitrawog commented on Oct 24, 2026

    @aitrawog
    ContributorAuthor

    STATUS — the DATA channel is implemented, gated and open as a PR.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

    Head. 253120997a33e1c021fb7c88609a78ef84f8b1de on feat/stepenv-data-channel, opened as opencharly/plugin-task#47. The diff is 7 files, +401 −45: the runner's seam, the SDK/spec pin bump, the unit tests, the README section, the live gate's new arm, and the coherence control's reworded header.

    What the change does. A task's params: are substituted into a plan step's text, and the shell then parses that text, so a value that is data can only be delivered as source. This change opens the other route: the task's RESOLVED values — its vars, its env, its resolved params and TASK_DIR — are handed to the SDK as kit.RunnerConfig.StepEnv, and the SDK's kit.MergeStepEnv merges them into every step's process environment, where the shell never re-parses them. Three rules bound the channel, and all three are the SDK's own merge rules, not a second implementation here: the step's own env: wins a collision; a name outside [A-Za-z_][A-Za-z0-9_]* is rejected, returned and disclosed as a NOTE on the step's message; and a piped stdin is not exported (the map is cloned before captureStdin mutates it, because the command verb renders an exported value into the command TEXT and a NUL byte would make the exec un-runnable). A lowercase param becomes reachable by name, which the expander's uppercase-only grammar could not do.

    Producer-first. The channel is an SDK capability (kit.StepEnvExporter, kit.RunnerConfig.StepEnv, kit.MergeStepEnv — opencharly/sdk#475, released as sdk v0.2026297.1851), so this module's require rises to it and the producer leg had to land first. It has: opencharly/charly#1062 merged as 87f6302456f9c821877e719cbe90615368cd85bc with the merge-time tag v2026.297.1957, and charly's main now pins sdk v0.2026297.1851 / spec v0.2026296.1036. This repo's own coherence control (scripts/check-coherence.sh, run by the coherence CI job on every PR) asserts that charly's pins LEAD this module's requires, and it is green against charly main.

    What is done. The implementation, the README section, the unit tests (the export, the collision rule, the disclosure of a non-exportable name, the stdin exclusion), the live gate's new arm (a real charly binary with this candy compiled in: the param arrives VERBATIM and the marker file does not exist — the silent outcome is asserted too), the R7 mutation proof (the arm's own verdict turns red without the channel and green on the restored tree), the module's local gate and the live gate — all on the committed tree this PR pushes. The full output is pasted in the PR body under How tested, and the body passes marketplace/scripts/pr_body_lint.py with 0 findings.

    What is next. The validator's verdict on #47, and then the legs: charly adopts this repo's tag, and the umbrella advances the plugin-task and charly gitlinks. Next update when the validator's report lands, or sooner if a finding needs a fix commit.

  4. aitrawog commented on Oct 24, 2026

    @aitrawog
    ContributorAuthor

    RESOLVED — the DATA channel landed, and the channel defect this issue names is closed.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

    The landing. The change is opencharly/plugin-task#47, squashed into main as b73907f1996dc2eec831550a4a10b8b16b9676dc at 2026-10-24T20:06:33Z, with the merge-time tag v2026.297.2006 and the module tag candy/plugin-task/v0.2026297.2006 on that commit, and the changelog entry CHANGELOG/2026.297.2006.md. charly/pr-validator returned PASS on the head the PR pushed, and the org's native auto-merge (squash) landed it.

    What the defect was, and what closed it. A task's params: were substituted into a plan step's TEXT, and the shell then PARSED that text, so a value that is data could only be delivered as source — a $(…) inside a value EXECUTED, and a lowercase param was unreachable from a step at all because the expander's grammar is uppercase-only. The task's RESOLVED values — its vars, its env, its resolved params and TASK_DIR — are now handed to the SDK as kit.RunnerConfig.StepEnv, and the SDK's kit.MergeStepEnv merges them into every step's PROCESS environment, where the shell never re-parses them. The step's own env: wins a collision; a name the shell cannot export is rejected, returned and disclosed as a NOTE on the step's message; and a piped stdin is not exported, because the export map is cloned before captureStdin mutates it (the command verb renders an exported value into the command TEXT, where a NUL byte would make the exec un-runnable).

    The proof. The repo's live gate grew an arm that runs a task whose param carries $(touch <marker>) against a real charly binary and asserts BOTH outcomes: the value arrives VERBATIM in the step's environment, AND the marker file does not exist — the silent outcome is the failure this channel exists to remove, so it is asserted too. The R7 mutation (dropping the param from the channel) turns that arm's own verdict red and the restored tree turns it green again. The unit tests cover the handover, the collision rule, the disclosure and the stdin exclusion.

    The residual. The validator's report on #47 left four minor observations; two of them are findings on the code that PR landed, and they are fixed in opencharly/plugin-task#48 (the stdin-exclusion test now asserts its precondition instead of only an absence, and every test pipe closes its read end). The other two are disposed of on the PR's thread: the gate arm's leftover file lives inside the gate's own work tree, which the gate removes on exit, and the dependency surface the SDK bump brings is the SDK's own (loaderkit → deploykit → buildkitdrive).

    The legs that follow. This repo is the consumer of the SDK's channel; charly is the consumer of this repo; the umbrella advances the gitlinks last. The order is — this repo's tag (written) → charly adopts it → the umbrella advances the plugin-task and charly gitlinks. The follow-up lands first, so charly pins the tag that carries the strengthened test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions