Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ inputs:
registry:
description: The URL of the npm registry to use for installing Utoo.
required: false
default: https://registry.npmjs.org/
default: https://registry.npmjs.org
cache-store:
required: false
type: boolean
Expand Down
2 changes: 1 addition & 1 deletion dist/setup/index.js

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion src/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ export type CacheState = {

export default async (options: Input): Promise<Output> => {
const version = options.version || "latest";
const registry = options.registry || "https://registry.npmjs.org/";
const registry = (options.registry || "https://registry.npmjs.org").replace(
/\/+$/,
"",
);
Comment on lines +39 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-critical critical

Security Vulnerability: Command Injection

The registry variable is constructed from user input (options.registry) and later passed directly to execSync with shell: true in setRegistry (line 207):

execSync(`ut config set registry ${registry} --global`, { ... })

Because shell: true is used, this creates a Command Injection vulnerability if a malicious or malformed registry URL is provided (e.g., containing shell metacharacters like ;, &, or |).

Recommendations:

  1. Input Validation (Defense-in-Depth): Validate that the registry is a valid URL before using it. This can be done by parsing it with new URL() which will throw an error for invalid URLs.
  2. Safe Execution (Primary Fix): Refactor setRegistry to use @actions/exec's getExecOutput (which is already imported) instead of child_process.execSync with shell: true. This avoids spawning a shell and safely passes arguments as an array.

For example, refactor setRegistry as follows:

async function setRegistry(registry: string): Promise<void> {
  try {
    await getExecOutput("ut", ["config", "set", "registry", registry, "--global"]);
  } catch (error) {
    warning(`Failed to set npm registry: ${error}`);
  }
}
Suggested change
const registry = (options.registry || "https://registry.npmjs.org").replace(
/\/+$/,
"",
);
const registryInput = options.registry || "https://registry.npmjs.org";
try {
new URL(registryInput);
} catch {
throw new Error(`Invalid registry URL: ${registryInput}`);
}
const registry = registryInput.replace(/\/+$/, "");

const utooCacheEnabled = isUtooCacheEnabled(options);
const storeCacheEnabled = options.cacheStore === true && isFeatureAvailable();

Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ if (!process.env.RUNNER_TEMP) {

runAction({
version: getInput("utoo-version") || "latest",
registry: getInput("registry") || "https://registry.npmjs.org/",
registry: getInput("registry") || "https://registry.npmjs.org",
cacheStore: getBooleanInput("cache-store") === true, // Default false
cacheUtoo: getBooleanInput("cache-utoo") === true, // Default true
})
Expand Down
Loading