From 3a88ac9696c292ebc4e6cb06a90ab412e2d7c296 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 5 Jul 2026 00:15:51 +0000
Subject: [PATCH 1/7] fix: restore 'empty version means latest' behavior broken
 by injection hardening

PR #44's command-injection hardening validated the caller-supplied
version with 'version !== undefined', but the public API documents
version as optional-defaults-to-latest and the pre-hardening code
treated any falsy version ('', null) as 'install latest'. Clients
sending an empty version field started getting
'Invalid package version: ...' errors from POST /packages/install and
PUT /packages/:name/upgrade.

Fixes:
- Normalize version input in installPackage/upgradePackage: null,
  empty, and whitespace-only versions are treated as undefined
  (latest), and surrounding whitespace is trimmed before validation.
  Malicious version specs are still rejected by the same allowlist.
- The npm package-name allowlist was also applied to python-runtime
  packages in upgradePackage and checkForUpdates before their python
  branches. PyPI names may legally contain uppercase letters (accepted
  at install time), so those records could never be upgraded or
  update-checked. Both paths now validate the persisted name against
  the allowlist matching the package's runtime.

Adds regression tests covering empty/null/whitespace versions on
install and upgrade, version trimming, and uppercase python package
names in upgradePackage/checkForUpdates.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015MvwXBH8tFAebZWZExXVm3
---
 src/services/packages.ts        |  66 +++++++++++---
 test/packages-injection.spec.ts | 155 ++++++++++++++++++++++++++++++++
 2 files changed, 207 insertions(+), 14 deletions(-)

diff --git a/src/services/packages.ts b/src/services/packages.ts
index 44f9a18..bc179b7 100644
--- a/src/services/packages.ts
+++ b/src/services/packages.ts
@@ -115,6 +115,29 @@ export class PackageService {
   // passed to `.cmd`/`.bat` files.
   private static readonly NPM_VERSION_SPEC_RE = /^(?!-)[A-Za-z0-9.\-+~^<>=|*x_]+$/
 
+  // Python (PyPI) package names: letters (either case), digits, `_`, `.`, `-`,
+  // starting with an alphanumeric. Mirrors the check applied at python
+  // install time so persisted names can be safely re-validated later.
+  private static readonly PYTHON_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/
+
+  // The public API documents `version` as "optional, defaults to latest".
+  // Callers commonly send `""` or `null` to mean "no specific version", and
+  // the pre-validation code treated any falsy version that way. Normalize
+  // those inputs to `undefined` (and trim whitespace) BEFORE validating, so
+  // the strict allowlist only ever rejects versions the caller actually set.
+  private static normalizeVersionInput(version: unknown): string | undefined {
+    if (version === undefined || version === null) {
+      return undefined
+    }
+    const text = typeof version === 'string' ? version : String(version)
+    const trimmed = text.trim()
+    return trimmed.length === 0 ? undefined : trimmed
+  }
+
+  private static isValidPythonPackageName(name: string): boolean {
+    return typeof name === 'string' && PackageService.PYTHON_NAME_RE.test(name)
+  }
+
   private static isValidNpmName(name: string): boolean {
     if (typeof name !== 'string' || name.length === 0 || name.length > 214) {
       return false
@@ -404,7 +427,6 @@ export class PackageService {
   }> {
     const {
       name,
-      version,
       serverName,
       transportType,
       command,
@@ -416,6 +438,8 @@ export class PackageService {
       enabled = true,
       failOnWarning = false
     } = request
+    // Empty/null versions mean "latest" (see normalizeVersionInput).
+    const version = PackageService.normalizeVersionInput(request.version)
     const resolvedTransportType: MCPTransportType = transportType ?? 'stdio'
     const runtime: PackageRuntime = request.runtime ?? 'node'
     const serverMetadata = this.buildServerMetadataInput(request)
@@ -427,7 +451,7 @@ export class PackageService {
       if (!/^[a-zA-Z0-9_.]+$/.test(request.pythonModule)) {
         return { success: false, error: `Invalid pythonModule: ${request.pythonModule}` }
       }
-      if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/.test(name)) {
+      if (!PackageService.isValidPythonPackageName(name)) {
         return { success: false, error: `Invalid Python package name: ${name}` }
       }
       if (resolvedTransportType !== 'stdio') {
@@ -828,11 +852,16 @@ export class PackageService {
           // Skip packages without mcpServerId
           if (!pkg.mcpServerId) continue
 
-          // Skip packages whose stored name would fail the strict allowlist.
-          // `pkg.name` originates from the install-time request and is
-          // re-validated here as defense-in-depth before it reaches the
-          // npm subprocess.
-          if (!PackageService.isValidNpmName(pkg.name)) {
+          // Re-validate the persisted name with the allowlist for the
+          // package's runtime before it reaches a subprocess. Python names
+          // may legally contain uppercase letters, so they must NOT be
+          // checked against the npm allowlist (that would skip legitimate
+          // python packages installed before this check existed).
+          const nameIsValidForRuntime =
+            pkg.runtime === 'python'
+              ? PackageService.isValidPythonPackageName(pkg.name)
+              : PackageService.isValidNpmName(pkg.name)
+          if (!nameIsValidForRuntime) {
             log({
               level: 'warn',
               msg: `Skipping update check for package with invalid name: ${pkg.name}`
@@ -921,6 +950,9 @@ export class PackageService {
     error?: string
   }> {
     try {
+      // Empty/null versions mean "latest" (see normalizeVersionInput).
+      version = PackageService.normalizeVersionInput(version)
+
       // Validate version against the same strict allowlist used for install,
       // since this value is passed verbatim to npm. Without this check, a
       // caller-controlled version would otherwise reach the npm command line.
@@ -937,11 +969,18 @@ export class PackageService {
         return { success: false, error: `Package ${serverName} not found` }
       }
 
-      // Re-validate the persisted package name. Although names are validated
-      // at install time, defending the npm invocation here makes the upgrade
-      // path safe even if a record predates the install-time check or was
-      // written through some other path.
-      if (!PackageService.isValidNpmName(packageInfo.name)) {
+      // Re-validate the persisted package name with the allowlist for the
+      // package's runtime. Although names are validated at install time,
+      // defending the subprocess invocation here makes the upgrade path safe
+      // even if a record predates the install-time check or was written
+      // through some other path. Python names may legally contain uppercase
+      // letters, so they are checked against the python allowlist, not npm's.
+      const packageRuntime: PackageRuntime = packageInfo.runtime ?? 'node'
+      const nameIsValidForRuntime =
+        packageRuntime === 'python'
+          ? PackageService.isValidPythonPackageName(packageInfo.name)
+          : PackageService.isValidNpmName(packageInfo.name)
+      if (!nameIsValidForRuntime) {
         return {
           success: false,
           error: `Invalid package name on existing record: ${packageInfo.name}.`
@@ -967,8 +1006,7 @@ export class PackageService {
       }
 
       try {
-        const runtime: PackageRuntime = packageInfo.runtime ?? 'node'
-        if (runtime === 'python') {
+        if (packageRuntime === 'python') {
           const venvAbsolutePath = packageInfo.venvPath
             ? path.resolve(process.cwd(), packageInfo.venvPath)
             : path.resolve(process.cwd(), packageInfo.installPath)
diff --git a/test/packages-injection.spec.ts b/test/packages-injection.spec.ts
index dc5ccf7..b854dd8 100644
--- a/test/packages-injection.spec.ts
+++ b/test/packages-injection.spec.ts
@@ -158,6 +158,54 @@ describe('PackageService command-injection hardening', () => {
 
       expect(result.success).toBe(true)
     })
+
+    // The API documents `version` as "optional, defaults to latest". Clients
+    // (including the GUI) send "" or null to mean "no specific version", and
+    // before the injection hardening any falsy version installed latest.
+    // These inputs must NOT be rejected by the version allowlist.
+    const emptyVersions: Array<[string, unknown]> = [
+      ['empty string', ''],
+      ['whitespace string', '   '],
+      ['null', null],
+      ['undefined', undefined]
+    ]
+    test.each(emptyVersions)('treats %s version as "install latest"', async (_label, empty) => {
+      const { service } = createService()
+
+      const result = await service.installPackage({
+        name: 'left-pad',
+        version: empty as string | undefined,
+        serverName: 'left-pad-server'
+      })
+
+      expect(result.success).toBe(true)
+
+      // The install spec must be the bare package name (no trailing @).
+      const installCall = execFilePromisifiedMock.mock.calls.find(([, args]) =>
+        Array.isArray(args) && (args as string[]).includes('install')
+      )
+      expect(installCall).toBeDefined()
+      const cmdArgs = installCall![1] as string[]
+      expect(cmdArgs[cmdArgs.length - 1]).toBe('left-pad')
+    })
+
+    test('trims surrounding whitespace from an otherwise valid version', async () => {
+      const { service } = createService()
+
+      const result = await service.installPackage({
+        name: 'left-pad',
+        version: ' 1.2.3 ',
+        serverName: 'left-pad-server'
+      })
+
+      expect(result.success).toBe(true)
+      const installCall = execFilePromisifiedMock.mock.calls.find(([, args]) =>
+        Array.isArray(args) && (args as string[]).includes('install')
+      )
+      expect(installCall).toBeDefined()
+      const cmdArgs = installCall![1] as string[]
+      expect(cmdArgs[cmdArgs.length - 1]).toBe('left-pad@1.2.3')
+    })
   })
 
   test('installPackage uses execFile (not shell exec) for npm install with package and version as separate spec arg', async () => {
@@ -354,6 +402,37 @@ describe('PackageService command-injection hardening', () => {
       const cmdArgs = installCalls[0][1] as string[]
       expect(cmdArgs).toEqual(['install', 'left-pad@latest'])
     })
+
+    test('upgradePackage treats empty-string version as "upgrade to latest"', async () => {
+      const { service, dbMock, mcpMock } = createService()
+      dbMock.findOne.mockResolvedValue({ ...existingPackage })
+
+      const serverShape = {
+        name: 'left-pad-server',
+        transportType: 'stdio',
+        command: 'node',
+        args: ['./packages/left-pad/node_modules/left-pad/index.js'],
+        env: {},
+        status: 'connected',
+        enabled: true
+      }
+      mcpMock.getServer.mockResolvedValue({ ...serverShape })
+      mcpMock.disableServer.mockResolvedValue({ ...serverShape, status: 'disconnected', enabled: false })
+      mcpMock.enableServer.mockResolvedValue({ ...serverShape })
+      mcpMock.updateServer.mockResolvedValue({ ...serverShape })
+      readFileMock.mockResolvedValue(JSON.stringify({ version: '2.0.0', main: 'index.js' }))
+
+      const result = await service.upgradePackage('left-pad-server', '')
+
+      expect(result.success).toBe(true)
+
+      const installCalls = execFilePromisifiedMock.mock.calls.filter(([, args]) =>
+        Array.isArray(args) && (args as string[]).includes('install')
+      )
+      expect(installCalls.length).toBeGreaterThan(0)
+      const cmdArgs = installCalls[0][1] as string[]
+      expect(cmdArgs).toEqual(['install', 'left-pad@latest'])
+    })
   })
 
   test('checkForUpdates uses execFile (not shell exec) for npm view', async () => {
@@ -457,6 +536,82 @@ describe('PackageService command-injection hardening', () => {
       expect(installCalls.length).toBe(0)
     })
 
+    test('checkForUpdates still checks python packages with uppercase names via pip', async () => {
+      // PyPI names may legally contain uppercase letters; they must not be
+      // filtered out by the npm name allowlist.
+      const { service, dbMock } = createService()
+      dbMock.find.mockResolvedValue([
+        {
+          name: 'MarkupSafe',
+          version: '2.0.0',
+          installPath: 'packages/python/markupsafe-server',
+          venvPath: 'packages/python/markupsafe-server',
+          status: 'installed',
+          installed: new Date('2025-01-01T00:00:00.000Z'),
+          mcpServerId: 'markupsafe-server',
+          enabled: true,
+          runtime: 'python'
+        }
+      ])
+
+      execFilePromisifiedMock.mockImplementation(async (...args: unknown[]) => {
+        const cmdArgs = (args[1] as string[]) ?? []
+        if (cmdArgs[0] === 'index' && cmdArgs[1] === 'versions') {
+          return { stdout: 'Available versions: 3.0.0, 2.0.0\n', stderr: '' }
+        }
+        return { stdout: '', stderr: '' }
+      })
+
+      const result = await service.checkForUpdates()
+
+      expect(result.updates).toEqual([
+        {
+          serverName: 'markupsafe-server',
+          currentVersion: '2.0.0',
+          latestVersion: '3.0.0',
+          updateAvailable: true
+        }
+      ])
+    })
+
+    test('upgradePackage allows python packages with uppercase names', async () => {
+      const { service, dbMock, mcpMock } = createService()
+      dbMock.findOne.mockResolvedValue({
+        name: 'MarkupSafe',
+        version: '2.0.0',
+        installPath: 'packages/python/markupsafe-server',
+        venvPath: 'packages/python/markupsafe-server',
+        status: 'installed',
+        installed: new Date('2025-01-01T00:00:00.000Z'),
+        mcpServerId: 'markupsafe-server',
+        enabled: false,
+        runtime: 'python',
+        pythonModule: 'markupsafe'
+      })
+      mcpMock.getServer.mockResolvedValue({
+        name: 'markupsafe-server',
+        transportType: 'stdio',
+        command: 'python',
+        args: ['-u', '-m', 'markupsafe'],
+        env: {},
+        status: 'disconnected',
+        enabled: false
+      })
+
+      execFilePromisifiedMock.mockImplementation(async (...args: unknown[]) => {
+        const cmdArgs = (args[1] as string[]) ?? []
+        if (cmdArgs[0] === 'show') {
+          return { stdout: 'Name: MarkupSafe\nVersion: 3.0.0\n', stderr: '' }
+        }
+        return { stdout: '', stderr: '' }
+      })
+
+      const result = await service.upgradePackage('markupsafe-server')
+
+      expect(result.success).toBe(true)
+      expect(result.package?.version).toBe('3.0.0')
+    })
+
     test('checkForUpdates skips packages with invalid persisted names', async () => {
       const { service, dbMock } = createService()
       dbMock.find.mockResolvedValue([

From e5a36c1ad8b6c4f80b3ad2d100856b4ddf744c9b Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 5 Jul 2026 00:28:39 +0000
Subject: [PATCH 2/7] chore: bump version to 1.11.11

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015MvwXBH8tFAebZWZExXVm3
---
 package.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package.json b/package.json
index 2761718..2110df4 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@missionsquad/mcp-api",
-  "version": "1.11.10",
+  "version": "1.11.11",
   "description": "MCP Servers exposed via HTTP API",
   "main": "dist/index.js",
   "repository": "missionsquad/mcp-api",

From 80f5f4efd460bd512d728ea5e4c7e4f0c8cdab8b Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 5 Jul 2026 00:42:49 +0000
Subject: [PATCH 3/7] fix: reject non-string version types instead of coercing
 to strings

Addresses review feedback on PR #46: normalizeVersionInput previously
stringified non-string inputs (true -> 'true', 123 -> '123'), which was
looser than the typeof guard in isValidNpmVersionSpec on main. Non-string,
non-null version values in the request body are now rejected with the
standard invalid-version error before normalization, in both
installPackage and upgradePackage.

Also per review: upgradePackage uses a new normalizedVersion const
instead of reassigning the parameter, and the pip index versions mock in
the uppercase-python test documents the output format the production
parser consumes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015MvwXBH8tFAebZWZExXVm3
---
 src/services/packages.ts        | 42 ++++++++++++++++++++-------
 test/packages-injection.spec.ts | 51 +++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/src/services/packages.ts b/src/services/packages.ts
index bc179b7..c6a7e10 100644
--- a/src/services/packages.ts
+++ b/src/services/packages.ts
@@ -125,15 +125,21 @@ export class PackageService {
   // the pre-validation code treated any falsy version that way. Normalize
   // those inputs to `undefined` (and trim whitespace) BEFORE validating, so
   // the strict allowlist only ever rejects versions the caller actually set.
-  private static normalizeVersionInput(version: unknown): string | undefined {
+  // Non-string types are NOT coerced — callers must reject them (see
+  // isVersionInputTypeValid) so `true`/`123` cannot sneak past the allowlist
+  // as stringified tags.
+  private static normalizeVersionInput(version: string | null | undefined): string | undefined {
     if (version === undefined || version === null) {
       return undefined
     }
-    const text = typeof version === 'string' ? version : String(version)
-    const trimmed = text.trim()
+    const trimmed = version.trim()
     return trimmed.length === 0 ? undefined : trimmed
   }
 
+  private static isVersionInputTypeValid(version: unknown): version is string | null | undefined {
+    return version === undefined || version === null || typeof version === 'string'
+  }
+
   private static isValidPythonPackageName(name: string): boolean {
     return typeof name === 'string' && PackageService.PYTHON_NAME_RE.test(name)
   }
@@ -438,7 +444,15 @@ export class PackageService {
       enabled = true,
       failOnWarning = false
     } = request
-    // Empty/null versions mean "latest" (see normalizeVersionInput).
+    // Reject non-string version types outright (booleans, numbers, objects
+    // in the JSON body) rather than coercing them to strings, then treat
+    // empty/null versions as "latest" (see normalizeVersionInput).
+    if (!PackageService.isVersionInputTypeValid(request.version)) {
+      return {
+        success: false,
+        error: `Invalid package version: ${String(request.version)}. Version must be a valid npm semver, range, or tag.`
+      }
+    }
     const version = PackageService.normalizeVersionInput(request.version)
     const resolvedTransportType: MCPTransportType = transportType ?? 'stdio'
     const runtime: PackageRuntime = request.runtime ?? 'node'
@@ -950,16 +964,24 @@ export class PackageService {
     error?: string
   }> {
     try {
-      // Empty/null versions mean "latest" (see normalizeVersionInput).
-      version = PackageService.normalizeVersionInput(version)
+      // Reject non-string version types outright (the controller passes
+      // req.body.version through without validating its type), then treat
+      // empty/null versions as "latest" (see normalizeVersionInput).
+      if (!PackageService.isVersionInputTypeValid(version)) {
+        return {
+          success: false,
+          error: `Invalid package version: ${String(version)}. Version must be a valid npm semver, range, or tag.`
+        }
+      }
+      const normalizedVersion = PackageService.normalizeVersionInput(version)
 
       // Validate version against the same strict allowlist used for install,
       // since this value is passed verbatim to npm. Without this check, a
       // caller-controlled version would otherwise reach the npm command line.
-      if (version !== undefined && !PackageService.isValidNpmVersionSpec(version)) {
+      if (normalizedVersion !== undefined && !PackageService.isValidNpmVersionSpec(normalizedVersion)) {
         return {
           success: false,
-          error: `Invalid package version: ${version}. Version must be a valid npm semver, range, or tag.`
+          error: `Invalid package version: ${normalizedVersion}. Version must be a valid npm semver, range, or tag.`
         }
       }
 
@@ -1011,7 +1033,7 @@ export class PackageService {
             ? path.resolve(process.cwd(), packageInfo.venvPath)
             : path.resolve(process.cwd(), packageInfo.installPath)
 
-          const spec = version ? `${packageInfo.name}==${version}` : packageInfo.name
+          const spec = normalizedVersion ? `${packageInfo.name}==${normalizedVersion}` : packageInfo.name
           await this.pipInstall(
             venvAbsolutePath,
             spec,
@@ -1047,7 +1069,7 @@ export class PackageService {
 
         // Perform the upgrade. Pass the package@version spec as a single argument
         // to execFile-based runNpm so shell metacharacters can never be interpreted.
-        const upgradeSpec = `${packageInfo.name}@${version ?? 'latest'}`
+        const upgradeSpec = `${packageInfo.name}@${normalizedVersion ?? 'latest'}`
         log({ level: 'info', msg: `Upgrading package: npm install ${upgradeSpec}` })
         const upgradeResult = await this.runNpm(['install', upgradeSpec], { cwd: packageDir })
 
diff --git a/test/packages-injection.spec.ts b/test/packages-injection.spec.ts
index b854dd8..7325a54 100644
--- a/test/packages-injection.spec.ts
+++ b/test/packages-injection.spec.ts
@@ -189,6 +189,54 @@ describe('PackageService command-injection hardening', () => {
       expect(cmdArgs[cmdArgs.length - 1]).toBe('left-pad')
     })
 
+    // Non-string version types must be rejected, not coerced to strings —
+    // `true` must never install the npm tag "true".
+    const nonStringVersions: Array<[string, unknown]> = [
+      ['boolean', true],
+      ['number', 123],
+      ['object', { version: '1.2.3' }],
+      ['array', ['1.2.3']]
+    ]
+    test.each(nonStringVersions)('rejects non-string version type (%s)', async (_label, bad) => {
+      const { service } = createService()
+
+      const result = await service.installPackage({
+        name: 'left-pad',
+        version: bad as unknown as string,
+        serverName: 'left-pad-server'
+      })
+
+      expect(result.success).toBe(false)
+      expect(result.error).toMatch(/version/i)
+      const installCalls = execFilePromisifiedMock.mock.calls.filter(([, args]) =>
+        Array.isArray(args) && (args as string[]).includes('install')
+      )
+      expect(installCalls.length).toBe(0)
+    })
+
+    test('upgradePackage rejects non-string version type', async () => {
+      const { service, dbMock } = createService()
+      dbMock.findOne.mockResolvedValue({
+        name: 'left-pad',
+        version: '1.0.0',
+        installPath: 'packages/left-pad',
+        status: 'installed',
+        installed: new Date('2025-01-01T00:00:00.000Z'),
+        mcpServerId: 'left-pad-server',
+        enabled: true,
+        runtime: 'node'
+      })
+
+      const result = await service.upgradePackage('left-pad-server', true as unknown as string)
+
+      expect(result.success).toBe(false)
+      expect(result.error).toMatch(/version/i)
+      const installCalls = execFilePromisifiedMock.mock.calls.filter(([, args]) =>
+        Array.isArray(args) && (args as string[]).includes('install')
+      )
+      expect(installCalls.length).toBe(0)
+    })
+
     test('trims surrounding whitespace from an otherwise valid version', async () => {
       const { service } = createService()
 
@@ -557,6 +605,9 @@ describe('PackageService command-injection hardening', () => {
       execFilePromisifiedMock.mockImplementation(async (...args: unknown[]) => {
         const cmdArgs = (args[1] as string[]) ?? []
         if (cmdArgs[0] === 'index' && cmdArgs[1] === 'versions') {
+          // Mirrors real `pip index versions <name>` output, which
+          // pipIndexLatestVersion parses by locating the line starting with
+          // "Available versions:" and taking the first comma-separated entry.
           return { stdout: 'Available versions: 3.0.0, 2.0.0\n', stderr: '' }
         }
         return { stdout: '', stderr: '' }

From fcf645f0318a16eef8b0f7e3babfacde6dfef37e Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 5 Jul 2026 00:46:19 +0000
Subject: [PATCH 4/7] docs: clarify intent of runtime version guard and
 PYTHON_NAME_RE scope

Per review on PR #46: note at the controller call site that the service's
version type/format re-validation is intentional defense-in-depth (the
body cast performs no runtime validation), and document that
PYTHON_NAME_RE is deliberately more permissive than PEP 508 since its
purpose is subprocess-argument safety, not PyPI name canonicalization.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015MvwXBH8tFAebZWZExXVm3
---
 src/controllers/packages.ts | 4 ++++
 src/services/packages.ts    | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/src/controllers/packages.ts b/src/controllers/packages.ts
index 4075d43..cba0cba 100644
--- a/src/controllers/packages.ts
+++ b/src/controllers/packages.ts
@@ -416,6 +416,10 @@ export class PackagesController implements Resource {
   private async upgradePackage(req: Request, res: Response, next: NextFunction): Promise<void> {
     try {
       const serverName = req.params.name
+      // The cast does not validate the body at runtime — PackageService
+      // deliberately re-checks the version's type and format (see
+      // isVersionInputTypeValid), so that guard is load-bearing, not
+      // redundant with this annotation.
       const version = req.body.version as string | undefined
 
       log({ level: 'info', msg: `Upgrading package ${serverName}${version ? ' to version ' + version : ''}` })
diff --git a/src/services/packages.ts b/src/services/packages.ts
index c6a7e10..620913c 100644
--- a/src/services/packages.ts
+++ b/src/services/packages.ts
@@ -118,6 +118,11 @@ export class PackageService {
   // Python (PyPI) package names: letters (either case), digits, `_`, `.`, `-`,
   // starting with an alphanumeric. Mirrors the check applied at python
   // install time so persisted names can be safely re-validated later.
+  // NOTE: intentionally more permissive than PEP 508, which also requires an
+  // alphanumeric final character and forbids consecutive separators (e.g.
+  // `my..pkg` passes here but is not a canonical PyPI name). Such names fail
+  // at pip-resolution time rather than here; the purpose of this allowlist is
+  // subprocess-argument safety, not PyPI canonicalization.
   private static readonly PYTHON_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/
 
   // The public API documents `version` as "optional, defaults to latest".

From 432584e8971ebfd891d4a46c8bfe0cca5321eb8d Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 5 Jul 2026 00:50:05 +0000
Subject: [PATCH 5/7] fix: enforce alphanumeric boundaries in python package
 names; strengthen pip-path test

Per review on PR #46:
- PYTHON_NAME_RE now requires names to end (as well as start) with an
  alphanumeric character, matching PEP 508's boundary rules, so names like
  'my-pkg-' get a structured validation error instead of raw pip stderr.
  Consecutive separators remain accepted (documented) since the allowlist
  exists for subprocess-argument safety, not PyPI canonicalization.
- The uppercase-python upgrade test now asserts the pip code path actually
  ran (pip install --upgrade MarkupSafe, version read via pip show) so a
  future refactor cannot pass the test through the node path vacuously.
- Added boundary-rejection tests for python install names.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015MvwXBH8tFAebZWZExXVm3
---
 src/services/packages.ts        | 16 ++++++++--------
 test/packages-injection.spec.ts | 16 ++++++++++++++++
 test/packages-python.spec.ts    | 17 +++++++++++++++++
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/src/services/packages.ts b/src/services/packages.ts
index 620913c..2c3707e 100644
--- a/src/services/packages.ts
+++ b/src/services/packages.ts
@@ -116,14 +116,14 @@ export class PackageService {
   private static readonly NPM_VERSION_SPEC_RE = /^(?!-)[A-Za-z0-9.\-+~^<>=|*x_]+$/
 
   // Python (PyPI) package names: letters (either case), digits, `_`, `.`, `-`,
-  // starting with an alphanumeric. Mirrors the check applied at python
-  // install time so persisted names can be safely re-validated later.
-  // NOTE: intentionally more permissive than PEP 508, which also requires an
-  // alphanumeric final character and forbids consecutive separators (e.g.
-  // `my..pkg` passes here but is not a canonical PyPI name). Such names fail
-  // at pip-resolution time rather than here; the purpose of this allowlist is
-  // subprocess-argument safety, not PyPI canonicalization.
-  private static readonly PYTHON_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/
+  // starting AND ending with an alphanumeric (single-character names allowed),
+  // matching PEP 508's boundary rules. Used both at python install time and to
+  // re-validate persisted names before they reach a pip subprocess.
+  // NOTE: still more permissive than full PEP 508 in that consecutive
+  // separators (e.g. `my..pkg`) are accepted; those fail at pip-resolution
+  // time. The purpose of this allowlist is subprocess-argument safety, not
+  // PyPI canonicalization.
+  private static readonly PYTHON_NAME_RE = /^[a-zA-Z0-9]([a-zA-Z0-9_.-]*[a-zA-Z0-9])?$/
 
   // The public API documents `version` as "optional, defaults to latest".
   // Callers commonly send `""` or `null` to mean "no specific version", and
diff --git a/test/packages-injection.spec.ts b/test/packages-injection.spec.ts
index 7325a54..c450aab 100644
--- a/test/packages-injection.spec.ts
+++ b/test/packages-injection.spec.ts
@@ -661,6 +661,22 @@ describe('PackageService command-injection hardening', () => {
 
       expect(result.success).toBe(true)
       expect(result.package?.version).toBe('3.0.0')
+
+      // Confirm the pip code path actually ran: the upgrade must go through
+      // `pip install --upgrade MarkupSafe` and the new version must have been
+      // read from `pip show MarkupSafe` (not from a node package.json).
+      const pipInstallCall = execFilePromisifiedMock.mock.calls.find(([, args]) =>
+        Array.isArray(args) &&
+        (args as string[])[0] === 'install' &&
+        (args as string[]).includes('--upgrade') &&
+        (args as string[]).includes('MarkupSafe')
+      )
+      expect(pipInstallCall).toBeDefined()
+      const pipShowCall = execFilePromisifiedMock.mock.calls.find(([, args]) =>
+        Array.isArray(args) && (args as string[])[0] === 'show'
+      )
+      expect(pipShowCall).toBeDefined()
+      expect(pipShowCall![1]).toEqual(['show', 'MarkupSafe'])
     })
 
     test('checkForUpdates skips packages with invalid persisted names', async () => {
diff --git a/test/packages-python.spec.ts b/test/packages-python.spec.ts
index 0998f14..6ea9779 100644
--- a/test/packages-python.spec.ts
+++ b/test/packages-python.spec.ts
@@ -116,6 +116,23 @@ describe('PackageService python runtime support', () => {
     expect(result.error).toBe('pythonModule is required for python runtime.')
   })
 
+  test.each(['trailing-dash-', 'trailing-dot.', 'trailing_underscore_', '-leading-dash'])(
+    'installPackage rejects python package name with non-alphanumeric boundary %p',
+    async (badName) => {
+      const { service } = createService()
+
+      const result = await service.installPackage({
+        name: badName,
+        serverName: 'python-server',
+        runtime: 'python',
+        pythonModule: 'my_mcp_server'
+      })
+
+      expect(result.success).toBe(false)
+      expect(result.error).toBe(`Invalid Python package name: ${badName}`)
+    }
+  )
+
   test('installPackage configures stdio server for python runtime with venv python -m module', async () => {
     const { service, dbMock, mcpMock } = createService()
 

From eab71406de3db5a68ed3161f0b2842d085e6d704 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 5 Jul 2026 00:54:15 +0000
Subject: [PATCH 6/7] chore: address review nits on version typing,
 diagnostics, and test robustness

Per review on PR #46:
- Controller cast widened to 'string | null | undefined' (JSON bodies can
  carry version: null) and upgradePackage's signature updated to match.
- upgradePackage logs a warn with the runtime when a persisted name fails
  its allowlist, and the checkForUpdates skip log includes the runtime,
  easing diagnosis of legacy records.
- Empty-string upgrade test locates the npm install call with find()
  instead of indexing [0], consistent with the rest of the suite.
- Documented in beforeEach why per-test execFile mockImplementations
  cannot leak across tests (mockResolvedValue replaces implementations).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015MvwXBH8tFAebZWZExXVm3
---
 src/controllers/packages.ts     |  5 +++--
 src/services/packages.ts        |  8 ++++++--
 test/packages-injection.spec.ts | 12 +++++++-----
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/controllers/packages.ts b/src/controllers/packages.ts
index cba0cba..2c16d86 100644
--- a/src/controllers/packages.ts
+++ b/src/controllers/packages.ts
@@ -419,8 +419,9 @@ export class PackagesController implements Resource {
       // The cast does not validate the body at runtime — PackageService
       // deliberately re-checks the version's type and format (see
       // isVersionInputTypeValid), so that guard is load-bearing, not
-      // redundant with this annotation.
-      const version = req.body.version as string | undefined
+      // redundant with this annotation. `null` is included because JSON
+      // bodies can carry {"version": null}.
+      const version = req.body.version as string | null | undefined
 
       log({ level: 'info', msg: `Upgrading package ${serverName}${version ? ' to version ' + version : ''}` })
 
diff --git a/src/services/packages.ts b/src/services/packages.ts
index 2c3707e..708442e 100644
--- a/src/services/packages.ts
+++ b/src/services/packages.ts
@@ -883,7 +883,7 @@ export class PackageService {
           if (!nameIsValidForRuntime) {
             log({
               level: 'warn',
-              msg: `Skipping update check for package with invalid name: ${pkg.name}`
+              msg: `Skipping update check for package with invalid ${pkg.runtime ?? 'node'} name: ${pkg.name}`
             })
             updates.push({
               serverName: pkg.mcpServerId,
@@ -961,7 +961,7 @@ export class PackageService {
    */
   async upgradePackage(
     serverName: string,
-    version?: string
+    version?: string | null
   ): Promise<{
     success: boolean
     package?: PackageInfo
@@ -1008,6 +1008,10 @@ export class PackageService {
           ? PackageService.isValidPythonPackageName(packageInfo.name)
           : PackageService.isValidNpmName(packageInfo.name)
       if (!nameIsValidForRuntime) {
+        log({
+          level: 'warn',
+          msg: `Refusing to upgrade ${serverName}: persisted package name ${packageInfo.name} fails the ${packageRuntime} name allowlist`
+        })
         return {
           success: false,
           error: `Invalid package name on existing record: ${packageInfo.name}.`
diff --git a/test/packages-injection.spec.ts b/test/packages-injection.spec.ts
index c450aab..e49b986 100644
--- a/test/packages-injection.spec.ts
+++ b/test/packages-injection.spec.ts
@@ -108,6 +108,9 @@ describe('PackageService command-injection hardening', () => {
     readFileMock.mockResolvedValue('{}')
     rmMock.mockResolvedValue(undefined)
     execPromisifiedMock.mockResolvedValue({ stdout: '', stderr: '' })
+    // mockResolvedValue REPLACES any per-test mockImplementation set by a
+    // previous test, so custom implementations (e.g. the pip index/show
+    // mocks below) cannot leak across tests.
     execFilePromisifiedMock.mockResolvedValue({ stdout: '', stderr: '' })
   })
 
@@ -474,12 +477,11 @@ describe('PackageService command-injection hardening', () => {
 
       expect(result.success).toBe(true)
 
-      const installCalls = execFilePromisifiedMock.mock.calls.filter(([, args]) =>
-        Array.isArray(args) && (args as string[]).includes('install')
+      const installCall = execFilePromisifiedMock.mock.calls.find(([, args]) =>
+        Array.isArray(args) && (args as string[]).includes('left-pad@latest')
       )
-      expect(installCalls.length).toBeGreaterThan(0)
-      const cmdArgs = installCalls[0][1] as string[]
-      expect(cmdArgs).toEqual(['install', 'left-pad@latest'])
+      expect(installCall).toBeDefined()
+      expect(installCall![1]).toEqual(['install', 'left-pad@latest'])
     })
   })
 

From 34ad78825ab6c78867f0a580e0a7c42cb46384c6 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 5 Jul 2026 00:57:28 +0000
Subject: [PATCH 7/7] docs: clarify separator deferral intent; trim version in
 upgrade log

Per review on PR #46:
- PYTHON_NAME_RE comment now states unambiguously that consecutive
  separators are intentionally deferred to pip (shell-inert via execFile
  argv, non-canonical per PEP 508), rather than framing it as a gap; the
  boundary tests carry a matching note.
- The controller's upgrade log trims the raw version so whitespace-only
  input no longer logs a misleading ' to version    ' suffix.

The empty-string upgrade test fixture already pins runtime: 'node'
explicitly, so no fixture change was needed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015MvwXBH8tFAebZWZExXVm3
---
 src/controllers/packages.ts  | 3 ++-
 src/services/packages.ts     | 9 +++++----
 test/packages-python.spec.ts | 3 +++
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/controllers/packages.ts b/src/controllers/packages.ts
index 2c16d86..04c9da2 100644
--- a/src/controllers/packages.ts
+++ b/src/controllers/packages.ts
@@ -423,7 +423,8 @@ export class PackagesController implements Resource {
       // bodies can carry {"version": null}.
       const version = req.body.version as string | null | undefined
 
-      log({ level: 'info', msg: `Upgrading package ${serverName}${version ? ' to version ' + version : ''}` })
+      const versionForLog = typeof version === 'string' ? version.trim() : ''
+      log({ level: 'info', msg: `Upgrading package ${serverName}${versionForLog ? ' to version ' + versionForLog : ''}` })
 
       const result = await this.packageService.upgradePackage(serverName, version)
       if (result.success) {
diff --git a/src/services/packages.ts b/src/services/packages.ts
index 708442e..c315084 100644
--- a/src/services/packages.ts
+++ b/src/services/packages.ts
@@ -119,10 +119,11 @@ export class PackageService {
   // starting AND ending with an alphanumeric (single-character names allowed),
   // matching PEP 508's boundary rules. Used both at python install time and to
   // re-validate persisted names before they reach a pip subprocess.
-  // NOTE: still more permissive than full PEP 508 in that consecutive
-  // separators (e.g. `my..pkg`) are accepted; those fail at pip-resolution
-  // time. The purpose of this allowlist is subprocess-argument safety, not
-  // PyPI canonicalization.
+  // Consecutive separators (e.g. `my..pkg`) are INTENTIONALLY accepted and
+  // deferred to pip for rejection: every character in such names is still
+  // shell-inert and passed via execFile argv, so they pose no
+  // subprocess-argument risk — they are merely non-canonical per PEP 508 and
+  // fail at pip-resolution time with pip's own error.
   private static readonly PYTHON_NAME_RE = /^[a-zA-Z0-9]([a-zA-Z0-9_.-]*[a-zA-Z0-9])?$/
 
   // The public API documents `version` as "optional, defaults to latest".
diff --git a/test/packages-python.spec.ts b/test/packages-python.spec.ts
index 6ea9779..06d38c3 100644
--- a/test/packages-python.spec.ts
+++ b/test/packages-python.spec.ts
@@ -116,6 +116,9 @@ describe('PackageService python runtime support', () => {
     expect(result.error).toBe('pythonModule is required for python runtime.')
   })
 
+  // Boundary rules only: consecutive separators (e.g. `my..pkg`) are
+  // intentionally NOT rejected here — they are shell-inert and deferred to
+  // pip for rejection (see PYTHON_NAME_RE in src/services/packages.ts).
   test.each(['trailing-dash-', 'trailing-dot.', 'trailing_underscore_', '-leading-dash'])(
     'installPackage rejects python package name with non-alphanumeric boundary %p',
     async (badName) => {