Fix code scanning alerts: workflow permissions and polynomial ReDoS - #37
Merged
Conversation
Add explicit least-privilege `permissions:` blocks to the three reusable-workflow callers that lacked one (actions/missing-workflow-permissions, alerts #1, #3, #4). A caller's permissions become the ceiling for the called workflow, so each grant covers exactly what the shared-config workflow needs: - cd.yml -> `contents: write`: shared-config's cd.yml checks out with persisted credentials and runs publish-rubygems-action, which does a raw `git push` of the release tag, then `gh release create`. - stale.yml -> `issues: write` + `pull-requests: write`: actions/stale comments on and closes both stale issues and stale PRs. - triage.yml -> `issues: write`: the callee runs `gh issue edit --add-label triage`; the callee's own job already declares `issues: write`, and the caller must not clamp below it. ci.yml and codeql.yml already declare permissions and are untouched. Fix the polynomial ReDoS in `Utils.underscore` (rb/polynomial-redos, alert #5). The acronym-splitting step `/([A-Z]+)([A-Z][a-z])/` is quadratic on long runs of uppercase letters that never reach the required uppercase-lowercase pair, since `[A-Z]+` can start matching at any offset within the run. Replacing it with the zero-width `/(?<=[A-Z])(?=[A-Z][a-z])/` and an `_` replacement is linear: the lookahead only matches immediately before the final uppercase of a run that is followed by a lowercase letter, and the lookbehind requires at least one preceding uppercase, which is exactly what `[A-Z]+` required. Equivalence was checked exhaustively over all strings up to length 6 from the alphabet [A B a b 1 - : _], over 200k random mixed-case strings up to length 24, and against the named cases now pinned in spec/lib/code_teams/utils_spec.rb. The other gsub/tr steps are unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add explicit least-privilege
permissions:blocks to the threereusable-workflow callers that lacked one
(actions/missing-workflow-permissions, alerts #1, #3, #4). A caller's
permissions become the ceiling for the called workflow, so each grant
covers exactly what the shared-config workflow needs:
contents: write: shared-config's cd.yml checks out withpersisted credentials and runs publish-rubygems-action, which does a
raw
git pushof the release tag, thengh release create.issues: write+pull-requests: write: actions/stalecomments on and closes both stale issues and stale PRs.
issues: write: the callee runsgh issue edit --add-label triage; the callee's own job alreadydeclares
issues: write, and the caller must not clamp below it.ci.yml and codeql.yml already declare permissions and are untouched.
Fix the polynomial ReDoS in
Utils.underscore(rb/polynomial-redos,alert #5). The acronym-splitting step
/([A-Z]+)([A-Z][a-z])/isquadratic on long runs of uppercase letters that never reach the
required uppercase-lowercase pair, since
[A-Z]+can start matching atany offset within the run. Replacing it with the zero-width
/(?<=[A-Z])(?=[A-Z][a-z])/and an_replacement is linear: thelookahead only matches immediately before the final uppercase of a run
that is followed by a lowercase letter, and the lookbehind requires at
least one preceding uppercase, which is exactly what
[A-Z]+required.Equivalence was checked exhaustively over all strings up to length 6
from the alphabet [A B a b 1 - : _], over 200k random mixed-case strings
up to length 24, and against the named cases now pinned in
spec/lib/code_teams/utils_spec.rb. The other gsub/tr steps are unchanged.
Alerts resolved
rb/polynomial-redos(high) —lib/code_teams/utils.rb:13actions/missing-workflow-permissions(medium) —.github/workflows/triage.yml:9actions/missing-workflow-permissions(medium) —.github/workflows/cd.yml:11actions/missing-workflow-permissions(medium) —.github/workflows/stale.yml:8Verification
permissions:block (cross-checked by parsing the YAML against the alert list).actionlintoutput is byte-identical tomain— no new findings introduced.codeql.ymluntouched.