Skip to content

docs(permissions): align PERMISSIONS.md with what the developer policy actually grants#1696

Merged
aidandaly24 merged 1 commit into
mainfrom
fix/permissions-doc-align
Jul 7, 2026
Merged

docs(permissions): align PERMISSIONS.md with what the developer policy actually grants#1696
aidandaly24 merged 1 commit into
mainfrom
fix/permissions-doc-align

Conversation

@jesseturner21

@jesseturner21 jesseturner21 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1683. docs/PERMISSIONS.md described the developer policy (docs/policies/iam-policy-user.json) as a narrow, least-privilege set, but the policy actually grants far more — including IAM role creation, cloudformation:*, and unscoped Secrets Manager access. This PR aligns the documentation with what the policy actually grants.

Root cause

The drift was introduced by #1092 ("fix: sync e2e IAM policy"), which merged the end-to-end test runner's permission set into the shipped customer-facing policy (+164 lines). That pulled in statements that were never documented:

  • HttpGatewayIamRoleManagementiam:CreateRole/DeleteRole/PutRolePolicy/PassRole on role/AgentCore-* (the escalation path the reporter flagged)
  • HarnessManagement / HarnessPassRoleiam:PassRole on role/*
  • CloudFormationFullcloudformation:* on *
  • SecretsManagersecretsmanager:GetSecretValue/CreateSecret/DeleteSecret on *
  • AgentCoreResourceManagement, CustomJwtCognitoSetup, ConfigBundleManagement
  • ImportTestIam / ImportTestPassRole / ImportTestS3 — pure e2e test scaffolding (role/bugbash-agentcore-role)

What this PR does (docs only)

  • Expands the high-level bullet list in §1 to include the resource-management, CloudFormation, IAM, Secrets Manager, and Cognito grants.
  • Adds action-by-action reference tables for every previously-undocumented statement (AgentCore resource management, configuration bundles, harnesses, full CloudFormation, HTTP gateway IAM role management, Custom JWT/Cognito, broad Secrets Manager, and the e2e-only test statements).
  • Extends the "Scoping down by feature" table so admins can trim these statements toward least privilege, with guidance to move resource-creating actions onto the CFN execution role (behind a permission boundary).

Testing

  • Verified every Sid in iam-policy-user.json is now represented in PERMISSIONS.md.
  • All internal anchor links resolve to existing headers.
  • Formatted with the repo's prettier settings (printWidth: 120, proseWrap: always).

@jesseturner21 jesseturner21 requested a review from a team July 7, 2026 13:47
@github-actions github-actions Bot added the size/m PR size: M label Jul 7, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 7, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.22.0.tgz

How to install

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

@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 7, 2026

@agentcore-cli-automation agentcore-cli-automation left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for putting together a thorough audit — the SID-by-SID mapping and the ⚠️ callout about privilege escalation are both really useful additions. One factual issue to fix before merge: the docs reference a destroy CLI command that doesn't exist (the CLI uses remove / remove all). See the two inline comments — they're the same nit in two places.

Everything else I spot-checked (all Sids present in iam-policy-user.json, every action in AgentCoreResourceManagement / HttpGatewayIamRoleManagement / CustomJwtCognitoSetup / HarnessManagement / ConfigBundleManagement / SecretsManager accounted for in the new reference tables, anchor links resolve including the two ### CloudFormation headers disambiguating cleanly, and the new BedrockModelInvocation rows match the policy) looked accurate.

Comment thread docs/PERMISSIONS.md
Comment thread docs/PERMISSIONS.md
@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 38.6% 14353 / 37179
🔵 Statements 37.91% 15298 / 40350
🔵 Functions 33.23% 2471 / 7436
🔵 Branches 32.35% 9563 / 29554
Generated in workflow #3991 for commit b3fcfcc by the Vitest Coverage Report Action

…n actually grants

The developer policy (docs/policies/iam-policy-user.json) drifted well
beyond what PERMISSIONS.md described, after #1092 synced the e2e test
runner's permission set into the shipped customer policy. Statements like
HttpGatewayIamRoleManagement (iam:CreateRole/PassRole on role/AgentCore-*),
CloudFormationFull (cloudformation:*), a broad SecretsManager grant on *,
AgentCoreResourceManagement, HarnessManagement/HarnessPassRole (iam:PassRole
on role/*), CustomJwtCognitoSetup, ConfigBundleManagement, and the ImportTest*
test statements were all undocumented.

This documents every statement the policy actually grants:
- Expands the high-level bullet list and adds an explicit warning that the
  policy is a permissive superset (shared with the e2e suite), not a
  least-privilege policy, and can enable privilege escalation via IAM role
  creation.
- Adds reference tables for AgentCore resource management, configuration
  bundles, harnesses, full CloudFormation, HTTP gateway IAM role management,
  Custom JWT/Cognito setup, the broad Secrets Manager grant, and the e2e-only
  test statements.
- Extends the 'Scoping down by feature' table so admins can trim the
  now-documented statements toward least privilege.

Closes #1683
@jesseturner21 jesseturner21 force-pushed the fix/permissions-doc-align branch from 4f9dd96 to b3fcfcc Compare July 7, 2026 14:08
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Jul 7, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 7, 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 7, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 7, 2026

@agentcore-cli-automation agentcore-cli-automation left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified this against origin/main's docs/policies/iam-policy-user.json:

  • All 31 Sids in the policy are either newly documented here or already covered by pre-existing sections (CloudFormationTemplateVerification is subsumed by the new CloudFormationFull coverage, and the duplicate BatchEvalAndRecommendation/BatchEvalAndRecommendations Sids are both covered by the existing Batch evaluation section).
  • Action lists in the new tables (AgentCoreResourceManagement, HttpGatewayIamRoleManagement, HarnessManagement/HarnessPassRole, ConfigBundleManagement, CustomJwtCognitoSetup, SecretsManager, ImportTest*) match the policy exactly, including resource scopes and the iam:PassedToService conditions.
  • Anchor links resolve. The two ### CloudFormation headings disambiguate cleanly — #cloudformation resolves to the CFN-execution-role narrow-actions section (which is the intended target of the "narrow cloudformation:* toward the specific actions listed under…" pointer at line 198), and the new full-lifecycle section gets #cloudformation-full-lifecycle.
  • The privilege-escalation callout accurately reflects that HttpGatewayIamRoleManagement (iam:CreateRole+iam:PutRolePolicy on role/AgentCore-*) combined with HarnessPassRole (iam:PassRole on role/*) is a real escalation path worth flagging.

No new serious issues from me. The one open thread I'd flag — the destroyremove factual issue in the CLI Commands column at line 551 — is already covered by the earlier inline comment from agentcore-cli-automation. The author's reply about CFN vocabulary is fair for the prose at line 52, but the column at line 551 is explicitly labeled "CLI Commands" and the CLI doesn't have a destroy command (there's even a regression test in src/cli/commands/deploy/__tests__/deploy-teardown.test.ts asserting this), so that one still stands. Deferring to the author on how to handle since it's already been raised.

Approving — docs-only, accurate, no blockers beyond what's already been surfaced.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 7, 2026
@aidandaly24 aidandaly24 merged commit 7080018 into main Jul 7, 2026
32 of 34 checks passed
@aidandaly24 aidandaly24 deleted the fix/permissions-doc-align branch July 7, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Developer Policy (docs/policies/iam-policy-user.json) grants many more permissions than described

3 participants