[Schema Consistency] Schema Consistency Check — 2026-06-09 #38059
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Schema Consistency Checker. A newer discussion is available at Discussion #38294. |
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
day_mod=0)role_name) is now fixed.This run prioritized re-verifying the high-signal standing findings from cache rather than re-enumerating fields. One previously-reported runtime bug has been resolved; one related behavioral/documentation gotcha remains; two long-standing low-priority inconsistencies persist.
Critical Issues
None that cause silent security bypass. The most impactful item is a behavioral semantics gap in
on.rolesmatching that can surprise users (see below). It fails closed (over-restrictive), so it is a usability/footgun issue rather than a security hole.✅ Resolved Since Last Run
on.rolesruntime now readsrole_name— Previouslycheck_permissions_utils.cjsread onlyrepoPermission.data.permission(which the GitHub API returns asadmin/write/read/none), making the granular enum valuesmaintain/triage/maintainereffectively dead. The current code atactions/setup/js/check_permissions_utils.cjs:242-251now readsrole_nameand prefers it:The
maintainer → maintainalias is also handled on both the required and effective side. Granular roles now function. 🎉Schema ↔ Documentation Mismatches
1.
on.rolesmatching is exact-equality, not a privilege threshold (undocumented gotcha)main_workflow_schema.json:1840-1854):rolesenum[admin, maintainer, maintain, write, triage, read, all], default[admin, maintainer, write].check_permissions_utils.cjs:252-256): matching is strict equality —normalizedRequired === effectiveRoleinside.some(...). There is no privilege hierarchy and no expansion in the compiler (confirmed — onlyskip-roles/rolesextraction exists, no level expansion).triggers.md:385— "based on repository permission level"compilation-process.md:217— "actor has admin/maintainer/write permission"roles: [write]intending "write or above" will have their admin and maintainer actors rejected, becauseadmin !== write. The default[admin, maintainer, write]masks this because it enumerates all three explicitly. This is a footgun: the natural mental model (higher roles satisfy a lower requirement) does not hold.rolesis an exact-match allowlist, not a minimum threshold; or (b) implement hierarchy expansion (write⇒ also acceptadmin,maintain).Evidence — exact-match logic
Schema ↔ Parser/Implementation Mismatches
2.
experiments.*.analysis_typeis a decorative enummain_workflow_schema.json:2875): enum[t_test, mann_whitney, proportion_test, bayesian_ab].pkg/cli/experiments_analyze_statistics.go): the value is stored (line 120,a.AnalysisType = cfg.AnalysisType) and printed to stderr (line 298,Test type : %s) only. The analyzer never branches on it — the onlyswitch(line 345) is ona.Recommendation, notAnalysisType.mann_whitneyorbayesian_abget t-test-equivalent behavior with no warning. The enum advertises capability the code doesn't deliver.Parser ↔ Documentation Mismatch
3.
user-invokable(IgnoredFrontmatterFields) is undocumentedpkg/constants/constants.go:315):IgnoredFrontmatterFields = []string{"user-invokable"}— silently accepted/ignored during frontmatter validation.docs/src/content/docs/**.Recommendations
on.rolesas exact-match (highest value) — add an explicit note intriggers.mdthatrolesis an allowlist, not a minimum threshold;roles: [write]does not include admins. Consider implementing hierarchy expansion if threshold semantics are intended.analysis_type— either implement the test-type branching or document which values are currently no-ops.user-invokable— brief mention in the frontmatter reference.Strategy Performance
Next Steps
on.rolesmatching semantics in docs (or add hierarchy expansion)analysis_typeenum valuesuser-invokableignored fieldReferences: §27188458090
Beta Was this translation helpful? Give feedback.
All reactions