Skip to content

Fix aws bundle allowlisting #763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adwsingh merged 1 commit into from
Jun 10, 2025
Merged

Fix aws bundle allowlisting #763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 14 additions & 19 deletions ...s/src/main/java/software/amazon/smithy/java/aws/mcp/cli/commands/AddAwsServiceBundle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package software.amazon.smithy.java.aws.mcp.cli.commands;

import java.util.Collections;
import java.util.Set;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;
Expand Down Expand Up @@ -36,18 +35,24 @@ public class AddAwsServiceBundle extends AbstractAddBundle {
protected Set<String> blockedApis;

@Option(names = "--include-write-apis",
description = "Include write APIs in the MCP server",
defaultValue = "false")
boolean includeWriteApis;
description = "Include write APIs in the MCP server")
Boolean includeWriteApis;

@Override
protected CliBundle getNewToolConfig() {
var bundleBuilder = AwsServiceBundler.builder()
.serviceName(awsServiceName)
.exposedOperations(allowedTools())
.blockedOperations(blockedTools());
if (!includeWriteApis) {
bundleBuilder = bundleBuilder.readOnlyOperations();
.serviceName(awsServiceName);
//If nothing is specified then default to only readOnlyOperations.
if (allowedApis != null) {
// User explicitly specified allowed APIs - use only those

@shwdean shwdean Jun 11, 2025

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.

Sorry just noticed this PR. I had previously designed the feature so that you could easily additionally include a small number of write APIs (or read APIs that don't follow the common naming scheme e.g. 'Query' and 'Scan' in DDB) without having to re-specify the entire list of read-only operations. With this change, you you need to individually allow every read API if you also want to use a single write API.

@adwsingh adwsingh Jun 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So the existing code didn't respect a users allowedApis flag, unless they explicitly pass a --include-write-apis=false . And if they don't pass it it automatically includes all APIs irrespective of the allowlist. TBH I think includeWriteApis is a poor choice of input. We should instead have had --readOnlyApis which adds all readOnlyApis and then --allowed lets you add more APIs and we can assume --readOnlyApis to be true by default if nothing is passed.

bundleBuilder.exposedOperations(allowedApis);
} else if (includeWriteApis == null || !includeWriteApis) {
// Default case or explicit false - read-only operations
bundleBuilder.readOnlyOperations();
}
// Always apply blocked operations if specified
if (blockedApis != null) {
bundleBuilder.blockedOperations(blockedApis);
}
var bundle = bundleBuilder.build().bundle();

Expand Down Expand Up @@ -76,14 +81,4 @@ protected String getToolBundleName() {
protected boolean canOverwrite() {
return overwrite;
}

@Override
protected Set<String> allowedTools() {
return allowedApis == null ? Collections.emptySet() : allowedApis;
}

@Override
protected Set<String> blockedTools() {
return blockedApis == null ? Collections.emptySet() : blockedApis;
}
}
8 changes: 4 additions & 4 deletions ...rc/main/java/software/amazon/smithy/java/aws/servicebundle/bundler/AwsServiceBundler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ public final class AwsServiceBundler extends ModelBundler {
private AwsServiceBundler(Builder builder) {
this.serviceName = builder.serviceName;
this.resolver = builder.resolver;
this.exposedOperations = builder.exposedOperations;
this.blockedOperations = builder.blockedOperations;
this.allowedPrefixes = builder.allowedPrefixes;
this.blockedPrefixes = builder.blockedPrefixes;
this.exposedOperations = builder.exposedOperations == null ? Collections.emptySet() : builder.exposedOperations;
this.blockedOperations = builder.blockedOperations == null ? Collections.emptySet() : builder.blockedOperations;
this.allowedPrefixes = builder.allowedPrefixes == null ? Collections.emptySet() : builder.allowedPrefixes;
this.blockedPrefixes = builder.blockedPrefixes == null ? Collections.emptySet() : builder.blockedPrefixes;
}

public static Builder builder() {
Expand Down
5 changes: 0 additions & 5 deletions mcp/mcp-cli-api/src/main/java/software/amazon/smithy/java/mcp/cli/AbstractAddBundle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package software.amazon.smithy.java.mcp.cli;

import java.util.Set;
import software.amazon.smithy.java.mcp.cli.model.Location;

/**
Expand Down Expand Up @@ -55,8 +54,4 @@ protected final Location getBundleFileLocation() {
* @return true if existing tool bundle can be overwritten, false otherwise
*/
protected abstract boolean canOverwrite();

protected abstract Set<String> allowedTools();

protected abstract Set<String> blockedTools();
}
11 changes: 0 additions & 11 deletions mcp/mcp-cli/src/main/java/software/amazon/smithy/java/mcp/cli/commands/AddSmithyBundle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package software.amazon.smithy.java.mcp.cli.commands;

import java.util.Set;
import picocli.CommandLine;
import picocli.CommandLine.Command;
import software.amazon.smithy.java.mcp.cli.AbstractAddBundle;
Expand Down Expand Up @@ -43,14 +42,4 @@ protected String getToolBundleName() {
protected boolean canOverwrite() {
throw new UnsupportedOperationException("Not implemented yet.");
}

@Override
protected Set<String> allowedTools() {
throw new UnsupportedOperationException("Not implemented yet.");
}

@Override
protected Set<String> blockedTools() {
throw new UnsupportedOperationException("Not implemented yet.");
}
}