-
Notifications
You must be signed in to change notification settings - Fork 33
chore: Migrate publishing to GHA, consolidate CI workflows #879
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.
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
base: master
Are you sure you want to change the base?
Conversation
.github/workflows/daily_ci.yaml
Outdated
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
To fix the problem, you should add an explicit permissions block to the workflow. Ideally, place it at the top/root level (directly under the name: and above the on: key) so it applies to all jobs not specifying their own permissions. If only read access to repository contents and metadata is required, set it as:
permissions:
contents: readIf your jobs require more privileges, you can expand this as needed. In this case, since jobs are not detailed and just call another workflow, it's safest to start with a minimal read permission as suggested. You should edit the .github/workflows/daily_ci.yaml file, after the name: key and before on:, to include this block.
-
Copy modified lines R3-R4
| @@ -1,5 +1,7 @@ | ||
| # This workflow runs every weekday at 15:00 UTC (8AM PDT) | ||
| name: Daily CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| schedule: |
.github/workflows/push.yaml
Outdated
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
To fix this, add a permissions: block at the workflow root (before on:) in .github/workflows/push.yaml to grant only the minimum permissions required for the jobs in this workflow. Since the workflow simply uses a reusable workflow, unless more granular permissions are needed, a starting point is to set contents: read, which is the most restrictive typical value for read-only operations. If more permissions are required by the reused workflow, those can be added (but based only on what is shown, start with contents: read). This is accomplished by editing .github/workflows/push.yaml and inserting the permissions block after the workflow name.
-
Copy modified lines R3-R4
| @@ -1,5 +1,7 @@ | ||
| # This workflow runs for every push to master | ||
| name: Push CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: [ubuntu-latest, windows-latest, macos-latest] | ||
| node: ["18.x", "20.x", "22.x", "latest"] | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js ${{ matrix.node }} | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ matrix.node }} | ||
| cache: 'npm' | ||
|
|
||
| # - name: Configure AWS Credentials for Tests | ||
| # uses: aws-actions/configure-aws-credentials@v4 | ||
| # with: | ||
| # aws-region: us-west-2 | ||
| # role-to-assume: arn:aws:iam::370957321024:role/GitHub-CI-MPL-Dafny-Role-us-west-2 | ||
| # role-session-name: JavaScriptTests | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci --unsafe-perm | ||
|
|
||
| - name: Build (for source code testing) | ||
| if: ${{ !inputs.test-published-packages }} | ||
| run: npm run build | ||
|
|
||
| # Run vector tests for all CI runs (Ubuntu only) | ||
| - name: Publish locally for vector tests | ||
| if: ${{ !inputs.test-published-packages && matrix.os == 'ubuntu-latest' }} | ||
| run: npm run verdaccio-publish | ||
|
|
||
| - run: npm test | ||
|
|
||
| - name: Run vector tests (local packages) | ||
| if: ${{ !inputs.test-published-packages && matrix.os == 'ubuntu-latest' }} | ||
| run: npm run verdaccio-verify-publish -- ci | ||
|
|
||
| # Run vector tests against published packages (release workflow validation, Ubuntu only) | ||
| - name: Run vector tests (published packages) | ||
| if: ${{ inputs.test-published-packages && matrix.os == 'ubuntu-latest' }} | ||
| run: npm run verdaccio-verify-publish -- public |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
The best way to fix this issue is to add a permissions block to the workflow to restrict the permissions of the GITHUB_TOKEN, adhering to the principle of least privilege. Because the workflow is for CI unit tests and does not appear to require write access to the repository, the minimal permission required is likely contents: read. This block can be added either at the workflow root (applies to all jobs unless overridden) or directly to the ci-unit-tests job. Adhering to current best practices, it's most appropriate to add it at the workflow top level, immediately after the name: line and before on:. No further code changes or external dependencies are required beyond this addition.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Shared CI Tests | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| workflow_call: |
Issue #, if available:
Description of changes:
local_verdaccio_publishscript, seems to fix some issuesI also learned that this package's tests don't run on Node 22.x or latest -- this might actually mean the package doesn't work for newer versions? Unsure.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.