Conversation
Records commands and the standing biome-update convention: plan $schema bump, verify schema on web, consider breaking changes, adapt config in a PR, downgrade + admin review if Biome still errors, verify with lint.
Summary - pin react and react-native peers to the Nitro 0.30 / RN 0.82 stack instead of wildcard ranges - bump biome to 2.5.9 and switch linter rules from recommended to preset - tighten brace-expansion, minimatch, ws, and yaml overrides to patched transitive versions - bump package version to 0.3.0 - image-size was evaluated: no patched release exists as of today (all versions <=2.0.2 still flagged via Metro); risk is low because exploitation would require file injection in CI at build Files: - in `package.json`: 0.3.0, biome 2.5.9, peer pins, extra ws/yaml overrides - in `package-lock.json`: lockfile for biome 2.5.9, ws 7.5.13, yaml 2.9.0, and peer metadata - in `biome.json`: schema 2.5.9 and `preset: "recommended"`
|
3 issues outstanding. See task
Code review: The bulk of this PR is a clean clang-format pass — I verified the reformatted crypto files (
Details are in the inline threads. No approval action taken. Reviewed a9c1c2a |
Modify the npm audit step to continue on error. This prevents the CI pipeline from failing due to known vulnerabilities in `image-size` (GHSA-w3rx-r6r6-pgpr, GHSA-5p2g-fcmc-qvqq) which currently lack a patched release. The risk is considered low as it is a build-time DoS vulnerability requiring file injection in the CI environment.
| return result; | ||
| } | ||
|
|
||
| std::vector<uint8_t> Hmac::sha256(const std::vector<uint8_t>& data) { |
There was a problem hiding this comment.
This PR adds ~220 lines of hand-rolled SHA-256/SHA-512/HMAC while libsodium is already a hard dependency of this library (linked unconditionally in android/CMakeLists.txt via FetchContent, and used directly in HybridConcealCrypto.cpp for secretbox/base64). libsodium provides vetted, constant-time implementations via crypto_auth_hmacsha256 / crypto_auth_hmacsha512 that would replace all of this code.
To be clear on correctness: I extracted sha256/sha512/hmacImpl from this file and verified them against FIPS 180-4 digest vectors and RFC 4231 HMAC test cases 1, 2, and 6 — all pass. The concern is risk posture and maintenance, not a current bug: hand-maintained hash primitives in a crypto library are easy to regress, and this PR simultaneously removes the jest test infrastructure, so nothing would catch a future edit breaking these functions.
Suggestion: either delegate to libsodium's crypto_auth_hmacsha256/crypto_auth_hmacsha512 (keeping the existing ArrayBuffer API), or keep the implementation and add the RFC 4231 vectors as unit tests.
| # (GHSA-w3rx-r6r6-pgpr, GHSA-5p2g-fcmc-qvqq; all versions <=2.0.2). | ||
| # Risk is low: build-time DoS that would require file injection in CI. | ||
| # Keep the report; do not fail the job until a fix exists. | ||
| - run: npm audit --audit-level=high |
There was a problem hiding this comment.
This PR removes jest, the test script, and the CI npm test step, but CLAUDE.md still documents npm test in three places, including the required quality gate npm run lint && npm run check && npm run cpp && npm test. After this change npm test fails with "Missing script: test", so the documented contribution workflow is broken.
If the goal was just to drop the unused jest dependency, consider a zero-dependency replacement (e.g. node --test) so the quality gate stays green — that would also give the new HMAC code (see the Hmac.cpp comment) a place for RFC 4231 vectors. Otherwise, update CLAUDE.md to remove the npm test references.
| { | ||
| "name": "react-native-conceal-crypto", | ||
| "version": "0.2.8", | ||
| "version": "0.3.0", |
There was a problem hiding this comment.
Version is bumped to 0.3.0 and two new public API methods (hmacSha256 / hmacSha512) are documented in the README, but CHANGELOG.md still ends at v0.2.8. The repo tracks every release in that table, so this release should get a v0.3.0 row (new features: hmacSha256/hmacSha512; also the RN ~0.82 peer pin is a notable compatibility change worth noting there).
No description provided.