Skip to content

ci: restrict workflow token permissions and add property-based tests - #1405

Open
mgarbs wants to merge 1 commit into
hiero-ledger:mainfrom
mgarbs:ci/openssf-scorecard-hardening
Open

ci: restrict workflow token permissions and add property-based tests#1405
mgarbs wants to merge 1 commit into
hiero-ledger:mainfrom
mgarbs:ci/openssf-scorecard-hardening

Conversation

@mgarbs

@mgarbs mgarbs commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description:

Raise the repository's OpenSSF Scorecard results without changing what any workflow or job does.

  • Declare a read-only top-level GITHUB_TOKEN in flow-pr-title-check.yaml and grant statuses: write only to the title-check job
  • Add fast-check as a devDependency and a property-based test suite for RetryUtils.retryTask

Related issue(s):

None (no tracking issue exists for this work).

Notes for reviewer:

Changes grouped by Scorecard check:

Token-Permissions

  • .github/workflows/flow-pr-title-check.yaml: the workflow had statuses: write at the top level, which Scorecard flags as an excessive default token. The top-level block is now contents: read and statuses: write moves to jobs.title-check.permissions, which is the only job (and the only scope) step-security/conventional-pr-title-action needs to post the commit status. Every other workflow already declares contents: read at the top level. This was the only top-level write scope the check penalises; the one other top-level write scope in the repository, id-token: write in publish-npm-package.yaml, is not scored (see below).

Fuzzing

  • package.json / package-lock.json: add fast-check (devDependency only, installed with npm 10 so the lockfile diff is limited to fast-check and its single dependency pure-rand).
  • test/unit/utils/RetryUtils.property.spec.ts: three property-based tests for RetryUtils.retryTask covering, for arbitrary maxRetries and failure counts: the task result is returned once it succeeds before the budget is exhausted; the task is invoked exactly maxRetries times and the error is rethrown when it never succeeds; retries stop as soon as shouldRetry rejects an error. They are picked up by the existing npm run test glob and run with backOff: 0, so they add well under a second to the suite.
Check Before After (local scorecard)
Token-Permissions 9 10
Fuzzing 0 10
Pinned-Dependencies 9 9
Binary-Artifacts 10 10
Dangerous-Workflow 10 10

Intentionally not changed:

  • publish-npm-package.yaml line 99, npm install -g npm@11.7.0: Scorecard reports every npm install as unpinned, but this is a global tool upgrade (npm >= 11.5 is what enables trusted publishing) rather than a project dependency install, so there is no lockfile to switch it to npm ci. Left as is; it is the only remaining Pinned-Dependencies finding.
  • SAST / CodeQL: CodeQL default setup is already enabled on this repository (the Analyze (actions) and Analyze (javascript-typescript) check runs on every commit), so no advanced codeql.yml workflow is added here. An advanced configuration cannot run alongside default setup, and Scorecard already credits the existing check runs.
  • flow-semantic-release.yml already scopes its write permissions at the job level and is untouched.
  • publish-npm-package.yaml keeps id-token: write at the top level. Scorecard does not penalise that scope, and the release workflow cannot be exercised from a pull request, so it is left as is.

Verification:

  • actionlint .github/workflows/* reports the same pre-existing notes as on main (self-hosted runner labels, existing shellcheck hints) and nothing new
  • scorecard --local . --checks Token-Permissions,Pinned-Dependencies,Binary-Artifacts,Dangerous-Workflow,SAST,Fuzzing before/after as in the table above
  • npm ci && npm run test on Node 20: 113 passing (110 existing + 3 new)

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@mgarbs
mgarbs requested review from a team as code owners August 27, 2026 14:56
@lfdt-bot

lfdt-bot commented Aug 27, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@swirlds-automation

swirlds-automation commented Aug 27, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Improve the repository's OpenSSF Scorecard results without changing any
job behaviour:

* Token-Permissions: declare a read-only top-level token
  (contents: read) in flow-pr-title-check.yaml and grant statuses: write
  only to the title-check job that needs it. This was the only top-level
  write scope that the Token-Permissions check penalises; the top-level
  id-token: write in publish-npm-package.yaml is not scored and is left
  untouched.
* Fuzzing: add fast-check as a devDependency and a property-based test
  suite for RetryUtils.retryTask (test/unit/utils/RetryUtils.property.spec.ts)
  that runs as part of the existing mocha unit tests.

Signed-off-by: Michael Garber <michael.garber@hashgraph.com>
@mgarbs
mgarbs force-pushed the ci/openssf-scorecard-hardening branch from 1593803 to f183a7b Compare August 27, 2026 15:07
@mgarbs

mgarbs commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Some context on the intent behind this PR: it's part of a coordinated effort to raise the OpenSSF Scorecard scores across the hiero-ledger org. Scorecard is an automated 0–10 rating of a repository's supply-chain security practices — workflow token permissions, dependency pinning, SAST coverage, committed binaries, fuzzing, and similar checks — and it's what the org's security dashboard tracks.

The checks this PR moves (measured with scorecard --local on this branch): Token-Permissions 9→10, Fuzzing 0→10. The published score is currently 7.2 and the projection once this merges and the weekly Scorecard cron re-runs is ≈7.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants