Aman-Mittal commented on PR #379:
URL: 
https://github.com/apache/fineract-backoffice-ui/pull/379#issuecomment-5308468986

   ## Correction: the tag trigger moved out of the CI workflow
   
   The first version of this PR added `tags: ['*']` to the CI push trigger so 
`check-version.mjs` would have a tag to compare against. **zizmor rejected it — 
eleven high-severity `cache-poisoning` findings, one per job.**
   
   The audit is right, and it is not a lint nit. A tag push is a publishing 
trigger; every CI job restores an npm cache through `actions/setup-node`; and a 
cache written from a lower-privilege context can then reach a release build. 
That is precisely the threat worth taking seriously in a release pipeline, so 
suppressing it would have been the wrong call.
   
   Attribution, checked rather than assumed — `zizmor --min-severity 
informational --min-confidence low` over `.github/workflows/`:
   
   ```
   main:            No findings to report.        (0)
   first attempt:   11 high  [cache-poisoning]
   after the fix:   No findings to report.        (0)
   ```
   
   **The fix:** the tag check moved to its own `Release Tag` workflow. It needs 
no dependencies — `check-version.mjs` imports only `node:fs`, `node:path` and 
`node:url` — so there is no `npm ci`, no cache, and nothing to poison.
   
   Dropping the `cache:` input was **not** sufficient: zizmor flags 
`actions/setup-node` as caching by default, because it caches the toolchain 
download regardless. The new workflow therefore has no `setup-node` at all and 
uses the Node the runner already provides. There is a comment saying so, 
because the way this regresses is somebody adding a dependency to that script 
and quietly pulling `npm ci` back into a tag build.
   
   `npm run check:version` still runs on every branch and pull request, so 
`0.0.0` and non-SemVer are still rejected there. Only the 
tag-versus-`package.json` comparison needed a tag to run on.
   
   ## CI status
   
   `Run zizmor` ✅ · `Apache RAT Scan` ✅ · `Dependency Integrity` ✅ · `CodeQL` ✅ 
· `Analyze (actions)` ✅ · `Verify Commit Signatures` ✅
   
   And the new job, which is the one that matters here — every assertion green 
on the first CI run:
   
   ```
   Container Image -> success
       ✅ Build the image
       ✅ Start Fineract for the container to talk to
       ✅ Run the image against it
       ✅ The application is served
       ✅ config.json carries every key the application reads
       ✅ The API is proxied, and answers as Fineract rather than as the 
application
       ✅ An API path ending in .js is proxied, not served from disk
       ✅ Security headers survive on a proxied response
   ```
   
   That is the check which would have caught the container serving its own HTML 
for every API call, now proving in CI what was previously only verified by hand.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to