Aman-Mittal commented on PR #695:
URL:
https://github.com/apache/fineract-backoffice-ui/pull/695#issuecomment-5971708968
Two things addressed since the last push.
## Rebased onto current `main`
#694 merged and touched the same import block in
`e2e/loan-flows-non-admin-roles.spec.ts` that this branch rewrote — it removed
`APIRequestContext`, while this branch replaced the whole block because those
helpers moved to `e2e/utils/ui-rbac.ts`. Resolved in favour of this branch's
version (`playwrightRequest`, `randomInt`, `Locator`, `API_BASE` and
`TENANT_ID` are genuinely unused in that file now), rebased, and force-pushed
with an explicit lease.
## CodeQL: disabling certificate validation — fixed, not dismissed
The scanner was right. `scripts/snapshot-permission-codes.mjs` set
`NODE_TLS_REJECT_UNAUTHORIZED = '0'`, which disables validation for the **whole
process**. Fineract's e2e container publishes 8443 only with the self-signed
certificate it generates for itself, so there is no CA to validate against —
but that is not a reason to leave that line in the tree, where it can be copied
somewhere it matters.
My first attempt scoped it to a single `https.Agent` and added a guard
refusing any non-loopback host, resolving the hostname first so `localhost`
could not be pointed elsewhere. That is genuinely narrower — an unvalidated
connection could not leave the machine — and I verified it refused
`example.com` and accepted `127.0.0.1`.
I threw it away. `rejectUnauthorized: false` is the same sink wherever it
sits, and a guard around it is still a bad line with a good comment.
**Splitting the two jobs removes it.** Obtaining the payload is the
operator's business and belongs in a shell script — which is where this repo
already does exactly this, against exactly this endpoint:
```bash
# scripts/e2e-stack.sh:54
curl -k -s -o /dev/null -w '%{http_code}' --max-time 5 "$HEALTH_URL"
```
So `scripts/fetch-permission-codes.sh` fetches and
`snapshot-permission-codes.mjs` normalises stdin. **No TLS configuration is
left in scanned code at all**, and the pair now works for any way of getting
the JSON: the local stack, a real instance with a proper chain
(`FINERACT_CA_CERT`, which drops `-k`), or a file saved earlier.
The normaliser is better for being pure — it validates what it is *given*
rather than what it fetched. Four failure modes now have their own messages,
each verified, with the committed snapshot intact afterwards:
| Input | Message |
| --- | --- |
| empty stdin | `Nothing on stdin. Pipe a GET /v1/permissions payload in: …`
|
| an HTML error page | `stdin was not JSON. First 200 characters: …` |
| a Fineract error object | `Expected an array of permissions. Got:
Insufficient privileges` |
| a truncated payload | `Only 1 codes came back … Refusing to overwrite the
snapshot` |
`npm run permissions:snapshot` is unchanged for the caller, and the round
trip still produces 723 codes with the five padded ones preserved verbatim.
This was the only such sink in the tree — `grep` for
`NODE_TLS_REJECT_UNAUTHORIZED` and `rejectUnauthorized` across `src`, `scripts`
and `e2e` now returns nothing outside prose. The e2e suite's own TLS relaxation
goes through Playwright's `ignoreHTTPSErrors`, which is a browser-context
option rather than a Node TLS setting.
--
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]
