fix bug with secret deletion

[j4ys0n]

No description provided.

[j4ys0n]

Automated review 🤖

Summary of Changes
This PR fixes a bug in secret injection during tool calls by scoping secrets to only those declared by the server via secretNames metadata, preventing unintended exposure of unrelated secrets. It also updates the package version to 1.10.1 and tightens validation in Secrets.delete() to allow deletion without requiring secretValue.

Key Changes & Positives

• Scoped secret injection in src/services/mcp.ts (hunk @@ -878,13 +878,34 @@) ensures only server-declared secrets (server.secretNames or legacy server.secretName) are injected, reducing attack surface and preventing accidental leakage of unrelated user secrets. 🟢
• Backward compatibility preserved: servers without secretNames still receive all secrets with a deprecation warning, easing migration.
• Validation logic in src/services/secrets.ts (hunk @@ -57,7 +57,11 @@) correctly allows secret deletion without secretValue, fixing a potential bug where deletion would incorrectly fail.

Potential Issues & Recommendations

• Issue / Risk: Legacy fallback in mcp.ts logs a warning but continues injecting all secrets for servers missing secretNames, which may delay migration and retain security risk for older servers.
• Impact: Unmigrated servers remain vulnerable to secret over-injection until they adopt secretNames.
• Recommendation: Consider adding a config flag or telemetry to track legacy usage and plan deprecation timeline; consider enforcing secretNames in future major version.
• Status: 🟡 Needs review

Language/Framework Checks

• TypeScript type safety maintained: server.secretNames ?? (server.secretName ? [server.secretName] : []) safely handles both array and legacy scalar fields.
• Logging levels appropriately updated: info for scoped secrets, warn for legacy fallback—consistent with severity of behavior change.

Security & Privacy

• Reduces secret exposure surface by enforcing least-privilege injection per server, aligning with principle of minimal disclosure.
• Prevents accidental leakage of secrets not intended for a given server (e.g., AWS keys injected into a GitHub tool), mitigating credential sprawl risk.

Approval Recommendation
Approve

• Merge after verifying secretNames metadata is documented and available for all new/updated MCP servers.
• Monitor logs for legacy fallback warnings post-deployment to identify servers needing migration.

[Copilot]

Pull request overview

This pull request fixes a bug in secret deletion validation and adds scoped secret injection for improved security. The changes ensure that the secretValue parameter is no longer required when deleting secrets, and that servers only receive secrets they explicitly declare rather than all available secrets.

Changes:

• Fixed validation logic in updateSecret to make secretValue optional for delete operations
• Implemented scoped secret injection so servers only receive declared secrets
• Added backward compatibility for servers without declared secret names
• Bumped version to 1.10.1

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/services/secrets.ts	Fixed validation to allow secret deletion without requiring secretValue parameter
src/services/mcp.ts	Added scoped secret injection logic that filters secrets based on server's declared secretNames
package.json	Version bump from 1.10.0 to 1.10.1 for patch release

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When a server declares secretNames but none of the declared secrets exist in the user's secrets store, scopedSecrets will be an empty object, and no log message indicates this scenario. This could make it difficult to debug why a server isn't receiving expected secrets. Consider adding a debug or info log message when a server declares secrets but none are found, to help with troubleshooting.

Suggested change

	args = { ...args, ...scopedSecrets }
	log({
	level: 'info',
	msg: `Scoped secrets applied to tool call - ${serverName}:${methodName} - ${Object.keys(scopedSecrets).join(', ')}`
	})
	const scopedSecretKeys = Object.keys(scopedSecrets)
	if (scopedSecretKeys.length > 0) {
	args = { ...args, ...scopedSecrets }
	log({
	level: 'info',
	msg: `Scoped secrets applied to tool call - ${serverName}:${methodName} - ${scopedSecretKeys.join(', ')}`
	})
	} else {
	// Server declared secrets, but none were found in the user's secrets store.
	log({
	level: 'info',
	msg: `Server ${serverName} declares secrets (${serverSecretNames.join(', ')}) but none were found in secrets store for user ${username}`
	})
	}