Skip to content

security: shell injection via unvalidated env var key in skill env injection (orchestrate.ts) #3269

@la14-1

Description

@la14-1

Severity: HIGH
File: `packages/cli/src/shared/orchestrate.ts` lines ~669-686

Description

The skill env var injection code builds shell `export` lines and interpolates them directly into a `printf` command via `runner.runServer()`:

```typescript
const envLines = skillEnvPairs
.map((pair) => {
const eqIdx = pair.indexOf("=");
if (eqIdx === -1) return "";
const key = pair.slice(0, eqIdx);
const val = pair.slice(eqIdx + 1);
return `export ${key}='${val.replace(/'/g, "'\\''")}'`;
})
.filter(Boolean)
.join("\n");
if (envLines) {
await asyncTryCatch(() =>
cloud.runner.runServer(`printf '\\n# [spawn:skills]\\n${envLines}\\n' >> ~/.spawnrc`),
);
}
```

Problems

  1. `key` is not validated — it could contain shell metacharacters, newlines, or backticks. The existing `generateEnvConfig()` validates keys against `/^[A-Z_][A-Z0-9_]*$/` and rejects null bytes in values, but this code path bypasses that validation entirely.
  2. Direct string interpolation — the `envLines` string is interpolated directly into the shell command string, unlike every other similar pattern in the codebase which uses base64 encoding (e.g. `injectEnvVarsToRunner`).

A malicious env var name (from a compromised manifest.json skill definition) like KEY\`rm -rf ~\` could execute arbitrary commands on the remote VM.

Recommendation

Apply the same base64-encode-and-decode pattern already used by `injectEnvVarsToRunner()` elsewhere in the file, and validate keys against `/^[A-Z_][A-Z0-9_]*$/` before processing.


-- refactor/security-auditor

Metadata

Metadata

Assignees

No one assigned

    Labels

    safe-to-workSecurity triage: safe for automated processingsecuritySecurity vulnerabilities and concerns

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions