mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
feat: Add proxy_install_only config to restrict proxy to download commands (#222)
* feat: Add proxy_install_only config to restrict proxy to download commands Introduces proxy_install_only (default: false) which, when enabled, skips the proxy for package manager commands that do not download packages (e.g. npm ls, pip list), avoiding unnecessary MITM overhead. - Add ProxyInstallOnly to Config and config template - Add IsKnownDownloadCommand / MayDownloadPackages to ParsedCommand - Add DownloadCommands to npm and pypi PM configs covering update, ci, audit, dlx, exec, x, download, run and equivalents per PM - Extract shared runner.Execute used by both proxy flow and guard - Proxy flow short-circuits to runner.Execute for non-download commands when proxy_install_only=true * refactor: Inject CommandExecutor into guard to fix dependency direction guard depended on internal/runner, which inverted the intended layer hierarchy. Now guard defines a CommandExecutor function type and accepts it as a constructor argument. internal/flows (the composition root) creates the executor closure wrapping runner.Execute and injects it, keeping guard free of internal/ dependencies. * refactor: Invert proxy_install_only logic to use known non-download commands Replace the DownloadCommands allowlist (opt-in, fail-open) with a NonDownloadCommands denylist (opt-out, fail-safe). The proxy now runs for all commands except those explicitly known to not download packages. Unknown or future package manager subcommands default to running with the proxy. Includes script runners (run, start, test, stop, restart) that can spin up local servers — setting proxy env vars on these breaks them without providing any security benefit. Also covers removal commands and local operations that never contact the registry. * fix: Support PMG_* env vars regardless of config file state AutomaticEnv only resolves env vars for keys Viper already knows about via AllKeys(). When a key is absent from the config file (commented out, new key added after last setup, or no config file at all), Viper had no knowledge of it and silently skipped the env var. Fix by registering all Config struct fields as Viper defaults via reflection (using mapstructure tags) before reading the config file. This ensures PMG_* env vars work in all cases. Precedence: cobra flags > env vars > config file > defaults. SetDefault is used (not Set) so env vars and config file can still override the Go defaults freely. Tests added covering all precedence levels including the key-absent- from-config-file case that was the original bug report. * fix: Only check first non-flag arg against NonDownloadCommands Scanning all args caused false proxy bypasses when package names or script arguments matched a NonDownloadCommands entry. For example: - npm exec test → "test" matched, proxy incorrectly skipped - npm update config → "config" matched, proxy skipped - npm publish --tag version → "version" matched, proxy skipped Fix by checking only the first non-flag argument (the subcommand). If it is not in NonDownloadCommands we break immediately, so trailing args never influence the classification. Applied to all four parsers: npm, pip/pip3, uv, and poetry. Regression tests added for the false positive cases. * refactor: Replace reflection-based Viper defaults with embedded template Load the embedded config template as the Viper base so all keys are registered upfront, enabling PMG_* env vars to work regardless of whether a key exists in the user's config file. * fix: Restore trusted_packages template entry and revert DefaultConfig change * docs: Document environment variable overrides for config keys * update npm test cmd * refactor: extract shared non-download command detection helper Replaces duplicated first-non-flag-arg detection loops in npm.go and pypi.go (pip + poetry parsers) with a shared isFirstNonFlagArgInList helper in packagemanager.go. https://claude.ai/code/session_01AHaKF3vc2Haj9tK3jgUBAs --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Sahil Bansal
+205
-2
@@ -71,12 +71,53 @@ func TestNpmParseCommand(t *testing.T) {
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "not an installation command",
|
||||
name: "update is not a known non-download command (proxy runs)",
|
||||
command: "npm update @types/node",
|
||||
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, parsedCommand)
|
||||
assert.Equal(t, 0, len(parsedCommand.InstallTargets))
|
||||
assert.Empty(t, parsedCommand.InstallTargets)
|
||||
assert.False(t, parsedCommand.IsKnownNonDownloadCommand)
|
||||
assert.True(t, parsedCommand.MayDownloadPackages())
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "ci is not a known non-download command (proxy runs)",
|
||||
command: "npm ci",
|
||||
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
|
||||
assert.NoError(t, err)
|
||||
assert.False(t, parsedCommand.IsKnownNonDownloadCommand)
|
||||
assert.True(t, parsedCommand.MayDownloadPackages())
|
||||
assert.False(t, parsedCommand.IsInstallationCommand())
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "audit is not a known non-download command (proxy runs; audit fix can download)",
|
||||
command: "npm audit",
|
||||
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
|
||||
assert.NoError(t, err)
|
||||
assert.False(t, parsedCommand.IsKnownNonDownloadCommand)
|
||||
assert.True(t, parsedCommand.MayDownloadPackages())
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "ls is a known non-download command (proxy skipped)",
|
||||
command: "npm ls",
|
||||
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, parsedCommand.IsKnownNonDownloadCommand)
|
||||
assert.False(t, parsedCommand.MayDownloadPackages())
|
||||
assert.False(t, parsedCommand.IsInstallationCommand())
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "install sets MayDownloadPackages via IsInstallationCommand",
|
||||
command: "npm install express",
|
||||
assert: func(t *testing.T, parsedCommand *ParsedCommand, err error) {
|
||||
assert.NoError(t, err)
|
||||
assert.False(t, parsedCommand.IsKnownNonDownloadCommand)
|
||||
assert.True(t, parsedCommand.IsInstallationCommand())
|
||||
assert.True(t, parsedCommand.MayDownloadPackages())
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -403,3 +444,165 @@ func TestBunParseCommand(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNpmProxyBehavior(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
pm func() (*npmPackageManager, error)
|
||||
command string
|
||||
isKnownNonDownloadCmd bool // proxy skipped when proxy_install_only=true
|
||||
isInstallationCommand bool
|
||||
}{
|
||||
// Commands that proxy MUST run for (not known-safe)
|
||||
{
|
||||
name: "yarn upgrade — proxy runs (may download)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultYarnPackageManagerConfig()) },
|
||||
command: "yarn upgrade",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "pnpm update — proxy runs (may download)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultPnpmPackageManagerConfig()) },
|
||||
command: "pnpm update",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "bun update — proxy runs (may download)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultBunPackageManagerConfig()) },
|
||||
command: "bun update",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "npm exec — proxy runs (may download and run package)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm exec create-react-app",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "pnpm dlx — proxy runs (downloads and runs package)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultPnpmPackageManagerConfig()) },
|
||||
command: "pnpm dlx create-react-app",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "pnpm exec — proxy runs (may resolve packages)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultPnpmPackageManagerConfig()) },
|
||||
command: "pnpm exec tsc",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "yarn dlx — proxy runs (downloads and runs package)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultYarnPackageManagerConfig()) },
|
||||
command: "yarn dlx create-react-app",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "bun x — proxy runs (bun's npx equivalent)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultBunPackageManagerConfig()) },
|
||||
command: "bun x create-vite",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
// Commands where proxy is safely skipped
|
||||
{
|
||||
name: "npm outdated — proxy skipped (read-only registry check)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm outdated",
|
||||
isKnownNonDownloadCmd: true,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "npm list — proxy skipped (lists installed packages)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm list",
|
||||
isKnownNonDownloadCmd: true,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "pnpm why — proxy skipped (dependency reason lookup)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultPnpmPackageManagerConfig()) },
|
||||
command: "pnpm why express",
|
||||
isKnownNonDownloadCmd: true,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "yarn why — proxy skipped (dependency reason lookup)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultYarnPackageManagerConfig()) },
|
||||
command: "yarn why express",
|
||||
isKnownNonDownloadCmd: true,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
// Script execution via run
|
||||
{
|
||||
name: "npm run dev — proxy skipped (executes local script, no registry contact)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm run dev",
|
||||
isKnownNonDownloadCmd: true,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "yarn run build — proxy skipped (executes local script)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultYarnPackageManagerConfig()) },
|
||||
command: "yarn run build",
|
||||
isKnownNonDownloadCmd: true,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "bun run test — proxy skipped (executes local script)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultBunPackageManagerConfig()) },
|
||||
command: "bun run test",
|
||||
isKnownNonDownloadCmd: true,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
// False positive regression: package/script names matching NonDownloadCommands words
|
||||
{
|
||||
name: "npm exec test — proxy runs (test is package arg, not subcommand)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm exec test",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "npm update config — proxy runs (config is package name, not subcommand)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm update config",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
{
|
||||
name: "npm publish --tag version — proxy runs (version is flag value, not subcommand)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm publish --tag version",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
// Unknown commands default to proxy running (fail safe)
|
||||
{
|
||||
name: "unknown npm subcommand — proxy runs (fail safe)",
|
||||
pm: func() (*npmPackageManager, error) { return NewNpmPackageManager(DefaultNpmPackageManagerConfig()) },
|
||||
command: "npm some-future-command",
|
||||
isKnownNonDownloadCmd: false,
|
||||
isInstallationCommand: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
pm, err := tc.pm()
|
||||
assert.NoError(t, err)
|
||||
|
||||
parsed, err := pm.ParseCommand(strings.Split(tc.command, " "))
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, tc.isKnownNonDownloadCmd, parsed.IsKnownNonDownloadCommand)
|
||||
assert.Equal(t, tc.isInstallationCommand, parsed.IsInstallationCommand())
|
||||
assert.Equal(t, !tc.isKnownNonDownloadCmd, parsed.MayDownloadPackages())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user