[Schema Consistency] Schema Consistency Check — 2026-06-08 #37770
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Schema Consistency Checker. A newer discussion is available at Discussion #38059. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Summary
This run took a fresh angle: rather than checking whether enum values are validated or branched on, it checked whether enum values can ever match the real data they are compared against at runtime. That surfaced a high-value issue in the
on.rolespermission gate where several documented enum values appear to be unreachable against GitHub's actual API contract.Critical Issues
🔴
on.rolesenum valuesmaintain/maintainer/triageare likely non-functional at runtimeThe schema (
pkg/parser/schemas/main_workflow_schema.json) and docs (docs/src/content/docs/reference/triggers.md:395) advertise the role valuesadmin, maintainer, maintain, write, triage, read, all. The runtime permission gate, however, only inspects thepermissionfield of the collaborator-permission API response and never readsrole_name:Per GitHub's documented contract,
GET /repos/{owner}/{repo}/collaborators/{username}/permissionreturnspermission∈admin/write/read/none, while the granular roles (maintain,triage, custom roles) are reported in the separaterole_namefield. If that contract holds:permission: "write"→roles: [maintain](or[maintainer]) never authorizes them.permission: "read"→roles: [triage]never authorizes them.requiredPerm === "maintainer" && permission === "maintain"alias branch (line 245) is therefore effectively dead code.Impact: A workflow author who writes
roles: [maintain]orroles: [triage](both schema-valid and documented) may silently get a gate that authorizes no one, or that behaves differently than the label implies — a security-relevant surprise.Evidence trail
properties.on.oneOf[1].properties.roles→[admin, maintainer, maintain, write, triage, read, all]triggers.md:395— "Available roles:admin,maintainer/maintain,write,triage,read,all."pkg/workflow/role_checks.go:30→GH_AW_REQUIRED_ROLESactions/setup/js/check_permissions_utils.cjs:16(parse) and:241-245(compare)grep role_name actions/setup/js/*.cjs→ no matches (the granular field is never read)maintain/triage:check_permissions.test.cjs:92,99🟠 Secondary:
rolesmatching is exact-equality, not a permission thresholdThe same
.some(... permission === requiredPerm ...)logic has no hierarchy. Narrowing the gate to a single lower level silently excludes higher-privileged actors:roles: [write]→ an admin actor reportspermission: "admin", which is!== "write"→ admin is rejected.The default
[admin, maintainer, write]masks this because it lists all three explicitly. But the docs describeon.rolesas filtering "based on repository permission level" (triggers.md:385), which naturally reads as a minimum threshold ("write or above"), not exact set-membership. This semantic is undocumented.Documentation Gaps
Standing items (re-confirmed, low priority)
IgnoredFrontmatterFields = ["user-invokable"](pkg/constants/constants.go:315) has zero mentions acrossdocs/. It is an intentionally-ignored GitHub Copilot custom-agent field, but its silent acceptance is undocumented.dispatch_repository/allowed_repositoriesusesnake_caseamong otherwisekebab-casesiblings (e.g.dispatch-workflow). Docs use snake_case consistently, so this is intentional; flagged only for naming-convention awareness.Schema Improvements Needed
maintain/triagefrom theon.rolesenum, or (b) keep them and fix the runtime check (see below). Leaving schema-valid-but-unreachable values is the worst outcome.Parser / Runtime Updates Required
actions/setup/js/check_permissions_utils.cjs:241-245— to honormaintain/triage, readrepoPermission.data.role_name(fall back topermission) and/or implement a proper permission hierarchy (admin > maintain > write > triage > read) so that requiring a lower role also admits higher ones.actions/setup/js/check_permissions.test.cjs:92,99— replace mockedpermission: "maintain"/"triage"with fixtures that match the real API shape (e.g.permission: "write", role_name: "maintain"), so tests reflect production behavior.Workflow Violations
None found. A sweep of
.github/workflows/*.mdtop-level keys against the schema surfaced only nested-key/expression false-positives from the pre-computedin_used_not_schemalist (e.g.judge_path,run_dir,serena-*tool names) — no real schema violations.Recommendations
getCollaboratorPermissionLevel: confirm whetherpermissioncan ever bemaintain/triage. This single check determines whether the critical finding is a real bug or a non-issue.role_nameand/or implement a permission hierarchy, and align the test fixtures with the real API shape.rolesmatching semantics (exact set-membership vs. threshold) intriggers.mdso authors understand thatroles: [write]does not implicitly includeadmin.experiments.*.analysis_typeenum (S20) and the silently-ignoreduser-invokablefield (S17).Strategy Performance
Next Steps
getCollaboratorPermissionLevelpermissionfield values against live GitHub APIrole_nameand/or a permission hierarchyon.rolesmatching semantics intriggers.mduser-invokabledocs) and S20 (analysis_typedecorative enum)References:
Beta Was this translation helpful? Give feedback.
All reactions