fix(platform): audit remediation for caching, rate limiting, and ops - #1568
Closed
CluelessBiker wants to merge 5 commits into
Closed
fix(platform): audit remediation for caching, rate limiting, and ops#1568CluelessBiker wants to merge 5 commits into
CluelessBiker wants to merge 5 commits into
Conversation
Harden the platform after a full-stack audit: invalidate route caches when schemas or custom endpoints change, fail closed on rate-limiter Redis errors, map ALREADY_EXISTS to 409, expose GET /live for health checks, and stop tracking docker/.env in git.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged incomplete sanitization in escapeRedisScanGlob — backslashes must be escaped before other glob metacharacters so SCAN match patterns stay literal.
Snyk runs pnpm test when package manifests change; the previous root script always exited 1, which failed PR checks after adding hermes tests.
Move glob escaping into StateManager.invalidateCacheIndexByPrefix so prefixes are sanitized where SCAN match patterns are built, avoiding duplicate escaping upstream.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Remove duplicate escapeRedisScanGlob, fix Redis Cluster scanStream typing, and build grpc-sdk in the root test script so Snyk's clean-checkout test run succeeds.
Contributor
|
This PR touches too many things and introducing inconsistencies. Route cache invalidation needs global handling and not just the DB CRUD routes. Rate limiting assumes (incorrectly) that redis will go down, which if it does, it introduces more problems than the rate limit. The assumption is that redis needs an HA deployment, not mitigation on the server side. 409 is OK, /live endpoint is redundant since /ready covers the case. Docker folder's .env is being used by existing scripts and this would break functionality, while the port changes will probably do the same |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up fixes from a full-stack audit of Conduit and Conduit-UI. This PR hardens the backend around caching, rate limiting, error responses, and local Docker setup.
ALREADY_EXISTSgRPC errors are mapped to HTTP 409 instead of a generic 500.GET /livehealth endpoint — Adds a lightweight liveness route for load balancers and ops checks.docker/.env(replaced with.env.example), fixes the authorization service port indocker-compose.yml, and ignores.pnpm-store/and build artifacts.Why
Stale route caches were the highest-risk finding: admin changes to schemas or endpoints could leave cached responses in place until TTL expiry. Rate-limiter bypass on Redis failure and missing health checks were medium-risk operational gaps. Tracking
docker/.envin git was a security hygiene issue.Related PRs
Test plan
GET /database/...responses refresh without waiting for TTLGET /liveon the admin API — expect 200docker compose upwith.envcopied from.env.example— authorization service starts on the correct port