feat: add EFS and S3 filesystem mount support (BYO agents and harness)#1436
Open
padmak30 wants to merge 1 commit into
Open
feat: add EFS and S3 filesystem mount support (BYO agents and harness)#1436padmak30 wants to merge 1 commit into
padmak30 wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
agentcore-harness-reviewing
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
Package TarballHow to installgh release download pr-1436-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.16.0.tgz |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @@ -1,4 +1,6 @@ | |||
| {{#if needsOs}} | |||
Contributor
Author
There was a problem hiding this comment.
These are false positives. The .py files under src/assets/python/ are Handlebars templates, not executable Python. The {{#if ...}}, {{#each ...}}, and {{/if}} directives are intentional — they are rendered at
project creation time by the Handlebars engine into valid Python. This pattern is pre-existing across all templates in the codebase (e.g., src/assets/python/http/strands/base/main.py).
| @@ -1,4 +1,6 @@ | |||
| {{#if needsOs}} | |||
| @@ -1,4 +1,6 @@ | |||
| {{#if needsOs}} | |||
| @@ -1,4 +1,6 @@ | |||
| {{#if needsOs}} | |||
| @@ -1,4 +1,6 @@ | |||
| {{#if needsOs}} | |||
| @@ -1,4 +1,6 @@ | |||
| {{#if needsOs}} | |||
| @@ -1,7 +1,65 @@ | |||
| {{#if needsOs}} | |||
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
github-actions
Bot
removed
the
agentcore-harness-reviewing
Adds session storage, EFS access point, and S3 Files access point
filesystem mounts across the full stack: CLI flags, TUI wizard steps,
schema validation, CDK IAM permissions, and generated agent templates.
CLI (agentcore create / add agent / add harness):
- --session-storage-mount-path, --efs-access-point-arn/--efs-mount-path,
--s3-access-point-arn/--s3-mount-path flags on create and add agent
- Harness create path wires filesystem flags through to harness.json
- Sync validation: ARN format, paired flags, max mounts, VPC requirement
in both validateCreateOptions and validateCreateHarnessOptions
- Async validation: L1 access point exists, L2 VPC/AZ topology, L3 SG
in agent create, add agent, and harness create paths
- Level 3 SG check uses EFS/S3 ARN region (not agent region) for mount
target SG queries; validation reads deployment region from aws-targets.json
TUI wizard:
- EFS/S3 two-step ARN→path entry with add/edit/remove review screens
- Shared useFilesystemMountState hook (generate wizard + BYO + harness)
- Shared buildMountListItems helper
- Session-storage advanced setting in harness wizard includes EFS/S3 steps
- VPC warning and validation on harness EFS/S3 ARN steps
- Harness TUI add flow forwards efsAccessPoints/s3AccessPoints to primitive
Schema:
- FilesystemConfigurationSchema union (sessionStorage | efsAccessPoint |
s3FilesAccessPoint) with z.strictObject, duplicate path detection,
max-count enforcement, VPC requirement
- EFS_ACCESS_POINT_ARN_PATTERN / S3_FILES_ACCESS_POINT_ARN_PATTERN
constants shared between CLI validators and Zod schema
- HarnessSpec gains efsAccessPoints/s3AccessPoints with VPC enforcement
and duplicate mount path validation
CDK / deploy:
- AgentCoreRuntime: typed filesystemConfigurations props (aws-cdk-lib 2.257)
- AgentCoreHarnessRole: EFS ClientMount/ClientWrite and S3 Files
ClientMount/ClientWrite IAM policies when mounts are configured
- harness-mapper writes all three filesystem types; hasFilesystem uses
correct boolean coercion; mount paths normalized (trailing slash stripped)
- Vended cdk-stack.ts and bin/cdk.ts include new HarnessConfig fields
Templates:
- HTTP, A2A, AGUI, MCP Python templates render file_read/file_write/
list_files filesystem tools via {{#if needsOs}} blocks
- needsOs uses || not ?? so S3-only agents correctly generate tools
- EFS ARN regex constants shared (single source of truth)
- regionFromEfsArn/regionFromS3FilesArn merged into single regionFromArn
Tests:
- filesystem-utils.test.ts: ARN format, path validation, pairing, mounts
- filesystem-roundtrip.test.ts, filesystem-error-quality.test.ts: schema
- harness-mapper.test.ts: EFS, S3, combined filesystem mapping
- validate.test.ts: 16 new EFS/S3 validation cases for create path
- harness-validate.test.ts: 12 new cases for harness create path
- buildMountListItems.test.ts: 6 cases for mount list item builder
- schema-mapper.test.ts: 12 filesystem configuration mapping cases
- useFilesystemMountState.test.tsx: 15 hook handler tests
- computeByoSteps.test.ts: filesystem step inclusion
- useGenerateWizard.test.tsx: EFS/S3 flow, edit/remove, deselect
padmak30
force-pushed
the
feat/byo-filesystem-new
branch
from
5e271d7 to
578752e
Compare
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
Coverage Report
|
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
notgitika
requested changes
Jun 2, 2026
| // non-fatal: Level 2 topology checks are skipped when VPC ID cannot be resolved | ||
| } | ||
| } | ||
| const fsValidation = await validateFilesystemMountsConfiguration({ |
Contributor
There was a problem hiding this comment.
style nit: this is duplicated across handleCreateCLI and handleCreateHarnessCLI + a bunch of other logic.
can we extract them to shared helpers?
| if not resolved.startswith(os.path.realpath(SESSION_STORAGE_PATH)): | ||
| raise ValueError(f"Path '{path}' is outside the storage boundary") | ||
| resolved = os.path.realpath(path) | ||
| if not any(resolved.startswith(os.path.realpath(m)) for m in _MOUNT_PATHS): |
Contributor
There was a problem hiding this comment.
If mount is /mnt/a, then /mnt/abc/secret passes the check because "/mnt/abc/secret".startswith("/mnt/a") is True, granting access outside the configured mount boundary.
Fix: if not any(resolved == os.path.realpath(m) or resolved.startswith(os.path.realpath(m) + os.sep) for m in _MOUNT_PATHS):
| .regex(/^\/mnt\/[^/]+$/, 'Must be a path under /mnt with exactly one subdirectory (e.g. /mnt/data)'), | ||
| .min(6) | ||
| .max(200) | ||
| .regex(/^\/mnt\/[a-zA-Z0-9._-]+\/?$/, 'Must be a path under /mnt with exactly one subdirectory (e.g. /mnt/data)'), |
Contributor
There was a problem hiding this comment.
same as CDK comment why did the regex change?
Contributor
Author
There was a problem hiding this comment.
Updated as per AWS docs https://docs.aws.amazon.com/cli/latest/reference/bedrock-agentcore-control/create-agent-runtime.html
Since deploy would have failed for invalid regex this should be a safe change for existing projects
Comment on lines
+402
to
+417
| /** | ||
| * Orchestrate full async filesystem validation (Levels 1–3). | ||
| * Returns on first error. Skips topology and inbound SG checks when agent VPC/SG info is absent. | ||
| */ | ||
| export async function validateFilesystemMountsConfiguration(opts: FilesystemValidationOptions): Promise<SyncResult> { | ||
| const { | ||
| efsMounts, | ||
| s3FilesMounts, | ||
| agentVpcId, | ||
| agentSubnetIds, | ||
| agentSecurityGroupIds, | ||
| region, | ||
| skipTopologyChecks, | ||
| skipInboundSgChecks, | ||
| } = opts; | ||
|
|
Contributor
There was a problem hiding this comment.
Nice 3 level validation here!
jariy17
reviewed
Jun 2, 2026
Contributor
jariy17
left a comment
jariy17
left a comment
There was a problem hiding this comment.
LGTM please modify e2e test to add these mounts.
| } | ||
|
|
||
| /** | ||
| * Orchestrate full async filesystem validation (Levels 1–3). |
Contributor
There was a problem hiding this comment.
nit: Add some information about how the validator works.
// L1: verify access point exists and agent SGs allow NFS egress (port 2049)
// L2: confirm mount target shares agent's VPC and has a subnet in a matching AZ
// L3: check mount target SGs permit inbound 2049 from agent SGs
// Each level is skipped if the prior level's prerequisite data is unavailable (graceful degradation)
| logger.logSubStep(`Type: ${addAgentConfig.agentType}, Language: ${addAgentConfig.language}`); | ||
|
|
||
| // Validate EFS/S3 filesystem mounts before writing anything (shared by create and BYO paths) | ||
| const validateFilesystemMounts = async () => { |
Contributor
There was a problem hiding this comment.
nit/p4: If a customer has multiple mounts, we should parallelize the validation by mount.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds session storage, EFS access point, and S3 Files access point filesystem mounts across the full stack: CLI flags, TUI wizard steps, schema validation, CDK IAM permissions, and generated agent templates.
CLI (agentcore create / add agent / add harness):
TUI wizard:
Schema:
CDK / deploy:
Templates:
Tests: