Skip to content

OS cmd injection fixed - #43

Merged
godronus merged 2 commits into
mainfrom
fix/ICM-50570
Aug 26, 2026
Merged

OS cmd injection fixed#43
godronus merged 2 commits into
mainfrom
fix/ICM-50570

Conversation

@godronus

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI lite review requested due to automatic review settings August 26, 2026 08:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens local workspace build and scaffolding tooling against OS command injection by avoiding shell-backed process execution in the Linux/Docker (production) path, and documents the incident and remediation in the project changelog.

Changes:

  • Switched npx invocations in JS/TS and AssemblyScript build flows from unconditional shell: true to shell: process.platform === "win32".
  • Updated scaffolding to call npx via execFile with an args array (instead of exec with a concatenated command string), with shell only on Windows.
  • Bumped @gcoredev/fastedge-sdk-js and @modelcontextprotocol/sdk dependency versions and added a detailed security entry in context/CHANGELOG.md.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/tools/local/workspace/compiler/jsBuild.ts Removes Linux/Docker shell execution from the JS/TS build npx spawn path.
src/tools/local/workspace/compiler/asBuild.ts Applies the same shell-avoidance pattern to AssemblyScript builds.
src/tools/local/scaffolding/scaffolds.ts Uses execFile("npx", args) to avoid interpolating user-controlled output paths into shell command strings.
package.json Updates key dependencies alongside the security fix.
context/CHANGELOG.md Documents root cause, fix strategy, and residual considerations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/tools/local/workspace/compiler/jsBuild.ts Outdated
Comment thread src/tools/local/workspace/compiler/asBuild.ts Outdated
Comment thread src/tools/local/scaffolding/scaffolds.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

package.json:24

  • Dependency versions were updated in package.json, but pnpm-lock.yaml is present in the repo and isn’t updated in this PR. This can lead to non-reproducible installs (and CI failures if installs are lockfile-frozen). Please regenerate and commit the updated lockfile for these version bumps.
  "dependencies": {
    "@gcoredev/fastedge-sdk-js": "^2.5.1",
    "@modelcontextprotocol/sdk": "^1.30.0",

@godronus
godronus requested a review from qrdl August 26, 2026 10:07
@godronus
godronus merged commit 1c25a85 into main Aug 26, 2026
4 checks passed
@godronus
godronus deleted the fix/ICM-50570 branch August 26, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants