Skip to content

fix(container): only ensure build-context .dockerignore after the Dockerfile is validated#1749

Open
aidandaly24 wants to merge 1 commit into
mainfrom
fix/preflight-ensure-dockerignore-after-validation
Open

fix(container): only ensure build-context .dockerignore after the Dockerfile is validated#1749
aidandaly24 wants to merge 1 commit into
mainfrom
fix/preflight-ensure-dockerignore-after-validation

Conversation

@aidandaly24

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1284 (merged). In the deploy preflight validateContainerAgents loop, warnDeprecatedBaseImage and ensureBuildContextDockerignore ran unconditionally after the Dockerfile existsSync check. With buildContextPath set and a missing/typo'd Dockerfile, preflight would write a .dockerignore into the build-context root even though the deploy aborts — dirtying the working tree on a failed deploy. Guard both behind an else branch so they run only once the Dockerfile is confirmed to exist.

Also rejects a dockerfile value of "." in isValidDockerfilePath (it names the build-context directory itself → a broken docker build -f .), keeping the guard in lockstep with the sibling @aws/agentcore-cdk schema.

Related Issue

Follow-up to #1284 (merged); no separate issue.

Documentation PR

N/A — no user-facing behavior change.

Type of Change

  • Bug fix

Testing

How have you tested the change?

  • I ran npm run test:unit
  • I ran npm run typecheck
  • I ran npm run lint

Added tests: the preflight ordering guard (ensure is skipped when the Dockerfile is missing), "." rejection, "./"-prefix acceptance, and the build-arg key length bound.

Checklist

  • I have added any necessary tests that prove my fix is effective or my feature works
  • My changes generate no new warnings

…idated

The preflight Container loop ran warnDeprecatedBaseImage and
ensureBuildContextDockerignore unconditionally, so with buildContextPath set and
a missing/typo'd Dockerfile it could write a stray .dockerignore into the repo on
a failing deploy (and risk masking the friendly "Dockerfile not found" error).
Guard both behind an else branch so they run only when the Dockerfile exists.

Also reject a dockerfile value of "." in isValidDockerfilePath (names the
build-context dir itself -> broken `docker build -f .`). Add tests: the preflight
ordering guard (ensure skipped when Dockerfile missing), "." rejection, "./"-prefix
acceptance, and the build-arg key >255 length bound (from the EnvVarNameSchema reuse).
@aidandaly24 aidandaly24 requested a review from a team July 13, 2026 18:16
@github-actions github-actions Bot added size/s PR size: S agentcore-harness-reviewing AgentCore Harness review in progress labels Jul 13, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 13, 2026
@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 13, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.24.0.tgz

How to install

gh release download pr-1749-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.24.0.tgz

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 39.76% 14755 / 37101
🔵 Statements 39.03% 15728 / 40288
🔵 Functions 33.9% 2517 / 7424
🔵 Branches 33.37% 9854 / 29523
Generated in workflow #4090 for commit bca9255 by the Vitest Coverage Report Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant