Skip to content

feat(mint): add e2e agent role for pool testing #2407

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
ifireball merged 6 commits into from
Jun 21, 2026
Merged

feat(mint): add e2e agent role for pool testing #2407

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8117e84
feat(mint): add e2e agent role for pool testing
ifireball Jun 18, 2026
2509f5e
test(config): expect e2e in ValidRoles
ifireball Jun 18, 2026
8ee2f1e
fix(forge): align e2e app manifest with mint role permissions
ifireball Jun 18, 2026
8c2ff27
chore(forge): gofmt AppPermissions struct alignment
ifireball Jun 18, 2026
aa108a2
test: add e2e role drift guards across mintcore and forge
ifireball Jun 18, 2026
01c65cf
fix(layers): skip harness wrapper for e2e mint role
ifireball Jun 21, 2026
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
4 changes: 4 additions & 0 deletions internal/cli/mint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,10 @@ func TestValidateMintSetupRole(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "coder", role)

role, err = validateMintSetupRole("e2e")
require.NoError(t, err)
assert.Equal(t, "e2e", role)

_, err = validateMintSetupRole("fix")
require.Error(t, err)
assert.Contains(t, err.Error(), "coder")
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type OrgConfig struct {

// ValidRoles returns the set of recognized agent roles.
func ValidRoles() []string {
return []string{"fullsend", "triage", "coder", "review", "fix", "retro", "prioritize"}
return []string{"fullsend", "triage", "coder", "review", "fix", "retro", "prioritize", "e2e"}
Comment thread
ifireball marked this conversation as resolved.
Resolved
Comment thread
ifireball marked this conversation as resolved.
Resolved
Comment thread
ifireball marked this conversation as resolved.
Resolved
Comment thread
ifireball marked this conversation as resolved.
Resolved

@fullsend-ai-review fullsend-ai-review Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] scope-ambiguity

Adding e2e to ValidRoles() makes it configurable in any org's defaults.roles or per-repo roles list via config YAML. The mint handler has ALLOWED_ROLES and ROLE_APP_IDS environment-variable controls per mint instance, but there is no code-level guard at the config validation layer restricting which orgs may reference the e2e role. Issue #2155 explicitly constrains: 'Limit minted e2e credentials to the dedicated test org pool.'

Suggested fix: Ensure the e2e role's GitHub App and PEM are deployed only to mint instances serving the dedicated test org pool, or add explicit org-allowlist checking for the e2e role.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] scope-inconsistency

The e2e role is added to ValidRoles() but intentionally excluded from DefaultAgentRoles() and PerRepoDefaultRoles(). ValidRoles() is used for config validation, but e2e is a pool/CI mint role that should never appear in org config files. The comment in harnesswrappers.go states 'The e2e role is a pool/CI mint role and is not installed as an agent app', but ValidRoles() is not documented as including non-agent roles. See also: [scope-ambiguity] finding at this location.

Suggested fix: Add a comment to ValidRoles() clarifying it includes mint-only roles that are not installed as agent apps.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] scope-inconsistency

The e2e role is added to ValidRoles() but intentionally excluded from DefaultAgentRoles() and PerRepoDefaultRoles(). ValidRoles() is used for config validation, but e2e is a pool/CI mint role. ValidRoles() itself lacks a comment noting it includes mint-only roles. The new TestValidRoles_RecognizedByMintcore test provides drift detection.

Suggested fix: Add a comment to ValidRoles() clarifying it includes mint-only roles that are not installed as agent apps.

}

// ValidProviders returns the set of recognized inference providers.
Expand Down
3 changes: 2 additions & 1 deletion internal/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ import (

func TestValidRoles(t *testing.T) {
roles := ValidRoles()
assert.Len(t, roles, 7)
assert.Len(t, roles, 8)
assert.Contains(t, roles, "fullsend")
assert.Contains(t, roles, "triage")
assert.Contains(t, roles, "coder")
assert.Contains(t, roles, "review")
assert.Contains(t, roles, "fix")
assert.Contains(t, roles, "retro")
assert.Contains(t, roles, "prioritize")
assert.Contains(t, roles, "e2e")
}

func TestPerRepoDefaultRoles(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions internal/dispatch/gcf/mintsrc/mintcore/github.go.embed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ var canonicalRolePermissions = map[string]map[string]string{
"retro": {"actions": "read", "contents": "read", "pull_requests": "write", "issues": "write", "metadata": "read"},
"prioritize": {"contents": "read", "issues": "write", "organization_projects": "write", "metadata": "read"},
"fullsend": {"actions": "write", "actions_variables": "read", "contents": "write", "pull_requests": "write", "workflows": "write", "metadata": "read"},
"e2e": {
"actions": "write", "actions_variables": "read", "administration": "write",
"contents": "write", "issues": "write", "members": "write", "metadata": "read",
"organization_administration": "write", "pull_requests": "write",
"secrets": "write", "workflows": "write",
},
}

// RolePermissions returns a deep copy of the role-to-permissions map,
Expand Down
6 changes: 6 additions & 0 deletions internal/mintcore/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ var canonicalRolePermissions = map[string]map[string]string{
"retro": {"actions": "read", "contents": "read", "pull_requests": "write", "issues": "write", "metadata": "read"},
"prioritize": {"contents": "read", "issues": "write", "organization_projects": "write", "metadata": "read"},
"fullsend": {"actions": "write", "actions_variables": "read", "contents": "write", "pull_requests": "write", "workflows": "write", "metadata": "read"},
"e2e": {
Comment thread
ifireball marked this conversation as resolved.
Resolved
Comment thread
ifireball marked this conversation as resolved.
Resolved
Comment thread
ifireball marked this conversation as resolved.
Resolved

@fullsend-ai-review fullsend-ai-review Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] permission-expansion

The new e2e role grants administration: write, organization_administration: write, secrets: write, and members: write — the most privileged GitHub App permissions available. No other role in the canonical permissions map requests organization_administration or secrets at write level. Combined, a minted e2e token could reconfigure repos, manage org settings, overwrite secrets, and modify org membership. The mint handler's ALLOWED_ROLES/ROLE_APP_IDS gating and exclusion from DefaultAgentRoles() are meaningful mitigating controls, but neither the code nor an ADR documents why each elevated permission is necessary, nor does the code enforce the pool-only constraint from issue #2155.

Suggested fix: For each permission beyond what the fullsend role already grants, document the specific e2e test operation that requires it (ideally in an ADR or inline comment). In particular: (1) Justify organization_administration: write — if tests only need repo-level admin, remove it. (2) Justify secrets: write — document whether tests must create/rotate secrets. (3) Justify members: write — if tests do not manage org membership, downscope to read. (4) Consider adding a code-level constraint restricting the e2e role to known test-pool orgs.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] permission-expansion

The new e2e role grants administration: write, organization_administration: write, secrets: write, and members: write — the most privileged GitHub App permissions available. No other role in the canonical permissions map requests organization_administration or secrets at write level. Combined, a minted e2e token could reconfigure repos, manage org settings, overwrite secrets, and modify org membership. The mint handler's ALLOWED_ROLES/ROLE_APP_IDS gating and exclusion from DefaultAgentRoles() are meaningful mitigating controls, but neither the code nor an ADR documents why each elevated permission is necessary, and no code-level constraint restricts the role to known test-pool orgs.

Suggested fix: For each permission beyond what the fullsend role already grants, document the specific e2e test operation that requires it (ideally in an ADR or inline comment). In particular: (1) Justify organization_administration: write — if tests only need repo-level admin, remove it. (2) Justify secrets: write — document whether tests must create/rotate secrets. (3) Justify members: write — if tests do not manage org membership, downscope to read. (4) Consider adding a code-level constraint restricting the e2e role to known test-pool orgs.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] permission-expansion

The new e2e role grants administration: write, organization_administration: write, secrets: write, and members: write — the most privileged GitHub App permissions available. No other role in the canonical permissions map requests organization_administration or secrets at write level. Combined, a minted e2e token could reconfigure repos, manage org settings, overwrite secrets, and modify org membership. The mint handler ALLOWED_ROLES/ROLE_APP_IDS gating and exclusion from DefaultAgentRoles() are meaningful mitigating controls, but neither the code nor an ADR documents why each elevated permission is necessary, and no code-level constraint restricts the role to known test-pool orgs.

Suggested fix: For each permission beyond what the fullsend role already grants, document the specific e2e test operation that requires it (ideally in an ADR or inline comment). In particular: (1) Justify organization_administration: write — if tests only need repo-level admin, remove it. (2) Justify secrets: write — document whether tests must create/rotate secrets. (3) Justify members: write — if tests do not manage org membership, downscope to read. (4) Consider adding a code-level constraint restricting the e2e role to known test-pool orgs.

"actions": "write", "actions_variables": "read", "administration": "write",
"contents": "write", "issues": "write", "members": "write", "metadata": "read",
"organization_administration": "write", "pull_requests": "write",
"secrets": "write", "workflows": "write",
},
}

// RolePermissions returns a deep copy of the role-to-permissions map,
Expand Down
2 changes: 1 addition & 1 deletion internal/mintcore/github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestCreateInstallationToken_UnknownRole(t *testing.T) {
}

func TestRolePermissions_AllRolesPresent(t *testing.T) {
expectedRoles := []string{"triage", "coder", "review", "fix", "retro", "prioritize", "fullsend"}
expectedRoles := []string{"triage", "coder", "review", "fix", "retro", "prioritize", "fullsend", "e2e"}
Comment thread
ifireball marked this conversation as resolved.
Resolved
allPerms := RolePermissions()
for _, role := range expectedRoles {
perms, ok := allPerms[role]
Expand Down
Loading