fix(security): remove exposed RPC credentials and operator-specific paths - #373
fix(security): remove exposed RPC credentials and operator-specific paths#373Bayyan16 wants to merge 1 commit into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78ac0fc5c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| secret-scan: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Make publishing depend on the secret scan
On a release: push, this new job runs independently while publish still has only needs: build-and-test; therefore the npm publish can start—and succeed—even when secret-scan has failed or is still running. This defeats the protection precisely for a release commit containing a credential in the package, so publish should depend on both jobs.
Useful? React with 👍 / 👎.
| docker run --rm | ||
| -v "$GITHUB_WORKSPACE:/repo" | ||
| zricethezav/gitleaks@sha256:c00b6bd0aeb3071cbcb79009cb16a60dd9e0a7c60e2be9ab65d25e6bc8abbb7f | ||
| dir /repo --redact --no-banner --max-archive-depth=1 |
There was a problem hiding this comment.
Scan Git history rather than only the checkout tree
This invokes Gitleaks' dir command, which scans files/directories rather than Git history (upstream command documentation); combined with the default shallow checkout, a PR can expose a secret in one commit, delete it before the tip, and pass this job even though the credential remains retrievable from the repository. The credentials removed by this commit are likewise still present in its parent, so use a history-capable gitleaks git scan with sufficient checkout depth (and rotate any already-exposed credentials).
Useful? React with 👍 / 👎.
Summary
scripts/entry points and the root-leveldeep-check.tsdiagnostic.Background
This addresses the code-remediation portion of #369 and incorporates the expanded scope identified in the first issue comment:
deep-check.ts;scripts/;sourcesContentmade the path part of packaged output.Both exposed credentials must still be treated as compromised. This PR removes them from the current tree; it does not rotate them or erase historical copies.
Changes
scripts/config.mjswith fail-fast validation forPERCOLATOR_RPC_URLandPERCOLATOR_ADMIN_KEYPAIR, using the existingdotenvdependency.deep-check.tsandfull-state.ts; no public-mainnet fallback remains in those entry points.../dist/index.jsand restored the scripts' removed keeper-fund derivation as a script-local helper without changing the SDK API.dist/; the source-map path is removed. The build also materializes pre-existing generated drift from current source indist/index.jsanddist/solana/stake.d.ts.percolatorct-sdk-4.3.0.tgz, whose embedded source map retained the operator path. Current package dry-run output remainsdist/-only..env.example, a precise.gitignoreexception, and clarification that filename ignores do not detect inline credentials.Security Properties
This does not claim that historical copies have been erased or that either credential has been rotated.
Validation
Validated with Node.js
v20.20.2and pnpm9.15.9(the downloaded Node archive matched the published SHA-256):pnpm install --frozen-lockfile— passed; lockfile was up to date, 152 packages installed, 2.3s.pnpm lint— passed.pnpm build— passed;dist/index.jsand source map regenerated successfully.pnpm test— passed. Vitest phase: 30 files passed; 1012 tests passed, 31 skipped (1043 total). All preceding directtsxtest stages also passed.PERCOLATOR_RPC_URL is not set.before side effects.PERCOLATOR_ADMIN_KEYPAIR is not set.before connection construction or filesystem reads.dirscan with redaction and--max-archive-depth=1— passed; approximately 4.19 MB scanned, no leaks found.os.homedir()assumption, absolute developer import, private-key marker, bearer token, or tracked private.envfile found.actionlintv1.7.12 (release archive checksum verified) — passed with exit 0.npm pack --dry-run --json— passed; 34 files, expecteddist/present, no.env, script, diagnostic, audit, or private configuration file included.git diff --check origin/main...HEAD— passed.git merge-tree --write-tree origin/main HEAD— passed with no conflict.The following repository checks require external sibling-repository artifacts and fail identically on
origin/mainat3704dfda7118f988a83459f18f999d177e7bc90f:pnpm verify-layout—percolator-prog/target/layout.jsonis absent.pnpm parity:check— expected sibling Rust parity binaries/state are absent and the matcher fixture already drifts.Neither failure is caused by this PR, and no ABI/layout/spec source is changed here.
Operational Follow-up Required
Risk and Compatibility
PERCOLATOR_RPC_URL; administrative scripts additionally requirePERCOLATOR_ADMIN_KEYPAIR.dotenvdevelopment dependency is reused.dist/after source changes; this removes the source-map path and synchronizes already-drifted generated output with current source.distoutputs; if it lands first,distshould be regenerated after rebasing rather than resolved mechanically.Issue Link
Addresses #369
The issue should remain open until authorized maintainers confirm provider-side rotation and decide/complete the appropriate history and artifact remediation.