fix(release): flag caller permission increases as breaking - #76
Conversation
Document the actions: write requirement for reusable review workflow callers and prepend migration guidance to release notes when caller-facing permissions increase.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: Checkout code | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| ref: ${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || '' }} | ||
|
|
||
| - name: Run release caller-permissions tests |
derekmisler
left a comment
derekmisler
left a comment
There was a problem hiding this comment.
nice work overall. the new caller-permissions module is well-structured and the test coverage is solid. a few small things worth considering:
| return LEVEL_RANK[direct] >= LEVEL_RANK[wildcard] ? direct : wildcard; | ||
| }; | ||
|
|
||
| const scopes = [...new Set([...Object.keys(previous), ...Object.keys(current)])].sort(); |
There was a problem hiding this comment.
suggestion (non-blocking): when previous has an explicit scope and current uses write-all (stored as {'*': 'write'}), the scopes union includes both * and the explicit key. diffCallerRequirements then flags both as increases, so the rendered warning has redundant bullets — one for * (which subsumes everything) and one for the explicit scope. probably worth filtering out explicit-scope entries after computing increases when a * entry already covers them (LEVEL_RANK[wildcard_from] <= LEVEL_RANK[inc.from] && LEVEL_RANK[wildcard_to] >= LEVEL_RANK[inc.to]).
| echo "ℹ️ First release — no previous requirement to compare against." | ||
| exit 0 | ||
| fi | ||
| PREV_FILE=/tmp/review-pr-previous.yml |
There was a problem hiding this comment.
suggestion (non-blocking): PREV_FILE is hardcoded to /tmp/review-pr-previous.yml. if two release jobs overlap on a shared or self-hosted runner (e.g. a stable and a beta release triggered close together), they race to overwrite each other's snapshot, which could produce a missed or spurious breaking-change warning. consider PREV_FILE=/tmp/review-pr-previous-${VERSION}.yml (and update the matching trap cleanup in tests/test-release-caller-permissions.sh).
| if (inner === '') return {}; | ||
| const permissions: PermissionsMap = {}; | ||
| for (const part of inner.split(',')) { | ||
| const m = part.trim().match(/^([A-Za-z][A-Za-z0-9_-]*):\s*(\S+)$/); |
There was a problem hiding this comment.
nitpick (super non-blocking): this regex rejects keys starting with _, but KEY_RE (used for block-map entries) allows [A-Za-z_] as the first character. github scope names don't use underscores today, but the inconsistency is worth aligning.
| const m = part.trim().match(/^([A-Za-z][A-Za-z0-9_-]*):\s*(\S+)$/); | |
| const m = part.trim().match(/^([A-Za-z_][A-Za-z0-9_-]*):\s*(\S+)$/); |
Move the PR-controlled shell test out of the workflow_run context and into the pull_request test workflow to resolve the CodeQL untrusted-checkout finding introduced by #76.
3 participants
Related Issues
Fixes #72
Summary
actions: writerequirement introduced in v2.0.3actions: readfail GitHub workflow validation before any job startsIssue expectations
actions: writenone < read < writeincreases and prepends migration guidanceValidation