Claude/sweet hawking s04z34#2354
Conversation
End-to-end proxy middleware that sits between organizations and AI APIs (Claude, OpenAI, Vertex AI, AWS Bedrock) to reduce token spend and quantify CO2 emissions avoided. What's included: - saas/server.js — Express server with proxy + API + static serving - saas/routes/proxy.js — Provider-agnostic AI API proxy with optimization pipeline - saas/routes/api.js — REST API for dashboard analytics - saas/lib/prompt-optimizer.js — Whitespace compression, filler removal, deduplication, truncation - saas/lib/cache-manager.js — In-memory semantic response cache - saas/lib/providers.js — Token pricing + CO2 estimates for all major providers - saas/lib/token-counter.js — Fast token estimation (cl100k heuristic) - saas/lib/db.js — JSON-backed analytics store (swap to SQLite/Redis for prod) - saas/public/index.html — Marketing landing page with live stats - saas/public/dashboard.html — Real-time analytics dashboard (Chart.js) - saas/public/css/main.css + dashboard.css — Dark-theme UI Key features: - Prompt compression saves 5-30% tokens per request with zero code changes - Semantic cache returns 100% savings on repeated prompts - Per-org cost and carbon tracking with live dashboard - Carbon savings quantified in gCO2 for ESG/sustainability reporting - One-line SDK integration: just change baseUrl Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011wxzf24d5rFJTDVjKzk6x1
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011wxzf24d5rFJTDVjKzk6x1
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a full TokenGuard SaaS app with shared runtime helpers, JSON analytics storage, Express API and proxy routes, deployment config, and two client-facing pages for the dashboard and landing site. ChangesTokenGuard SaaS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function extractOrgId(req) { | ||
| const key = req.headers['x-tokenguard-org'] || req.headers['authorization'] || 'default'; | ||
| return key.slice(0, 16).replace(/[^a-z0-9]/gi, '_'); | ||
| } |
There was a problem hiding this comment.
When no x-tokenguard-org header is provided, extractOrgId falls back to the Authorization header — which contains the client's API key (e.g., Bearer sk-proj-abc...). The first 16 characters of that string become the org ID key (after character replacement, something like Bearer_sk_proj_). This value is then written to data/analytics.json via db.recordRequest and returned to any caller via GET /api/orgs. Anyone who can reach the dashboard endpoint receives a prefix of every client's API key that didn't supply x-tokenguard-org.
| function save() { | ||
| // Keep only last 10k requests in file | ||
| if (_data.requests.length > 10000) _data.requests = _data.requests.slice(-10000); | ||
| fs.writeFileSync(DB_FILE, JSON.stringify(_data, null, 2)); | ||
| } |
There was a problem hiding this comment.
save() calls fs.writeFileSync synchronously inside recordRequest, which runs on the hot path for every proxied request. Node.js processes this on the main thread, so any concurrent request is stalled while the JSON file (potentially hundreds of KB) is serialized and flushed to disk. Under realistic load this will cause severe latency spikes and may make the proxy appear unresponsive. Use fs.writeFile (async) or batch writes on a short interval instead.
| function save() { | |
| // Keep only last 10k requests in file | |
| if (_data.requests.length > 10000) _data.requests = _data.requests.slice(-10000); | |
| fs.writeFileSync(DB_FILE, JSON.stringify(_data, null, 2)); | |
| } | |
| let _saveTimer = null; | |
| function save() { | |
| // Keep only last 10k requests in memory | |
| if (_data.requests.length > 10000) _data.requests = _data.requests.slice(-10000); | |
| // Debounce writes — flush at most once per second to avoid blocking the event loop | |
| if (_saveTimer) return; | |
| _saveTimer = setTimeout(() => { | |
| _saveTimer = null; | |
| fs.writeFile(DB_FILE, JSON.stringify(_data, null, 2), (err) => { | |
| if (err) console.error('[db] write error:', err.message); | |
| }); | |
| }, 1000); | |
| } |
| const cache = new Map(); // key → { response, hits, createdAt, expiresAt } | ||
| const DEFAULT_TTL_MS = 30 * 60 * 1000; // 30 minutes |
There was a problem hiding this comment.
TOKENGUARD_CACHE_TTL_MS env var is documented but never read
.env.example documents TOKENGUARD_CACHE_TTL_MS=1800000 and TOKENGUARD_AGGRESSIVE=false as configurable, but neither is consumed anywhere in the codebase. DEFAULT_TTL_MS is hardcoded to 30 * 60 * 1000, and aggressive in proxy.js only reads the per-request x-tokenguard-aggressive header. Operators who set these env vars will see no effect, making the documented configuration silently inoperable.
| // Detect provider from path prefix: /proxy/openai/v1/... or /proxy/anthropic/v1/... | ||
| function detectProvider(req) { | ||
| const parts = req.path.split('/').filter(Boolean); | ||
| return parts[0] || 'openai'; | ||
| } |
There was a problem hiding this comment.
detectProvider is dead code
This function is defined but never called — the route handler already extracts the provider from req.params.provider. It can be removed to avoid confusion.
| // Detect provider from path prefix: /proxy/openai/v1/... or /proxy/anthropic/v1/... | |
| function detectProvider(req) { | |
| const parts = req.path.split('/').filter(Boolean); | |
| return parts[0] || 'openai'; | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| const app = express(); | ||
| const PORT = process.env.PORT || 3000; | ||
|
|
There was a problem hiding this comment.
app.use(cors()) with no options defaults to allowing all origins. Since the proxy forwards requests to upstream AI APIs (including client-supplied Authorization headers), a malicious webpage on any domain can make cross-origin requests through this proxy on behalf of a visiting user's browser. For a self-hosted deployment, CORS should be restricted to known internal origins or disabled for non-browser use-cases.
| @@ -0,0 +1,2010 @@ | |||
| { | |||
There was a problem hiding this comment.
This file contains ~2,000 synthetic request records with realistic-looking org IDs (acme-corp, stripe-team, datadog-ml, uber-eng). Since db.js loads from this path on first access, every fresh deployment will start with pre-populated analytics instead of a clean slate. The seed data should either be removed or placed outside the data/ directory that db.js treats as its live store. Consider adding saas/data/analytics.json to .gitignore and replacing this with an empty init file.
Railpack was detecting Python from repo root. These config files tell Railway explicitly to use Node.js, run npm install, and start with node. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011wxzf24d5rFJTDVjKzk6x1
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@saas/lib/cache-manager.js`:
- Line 27: In the cache manager read/write path, the cached response is being
stored and returned by reference, which allows outside mutations to leak into
cache state, and the read path is spreading entry.response without guarding for
null or non-plain values. Update the cache-manager logic around the response
storage and retrieval helpers so cached entries are cloned into new objects when
written, and the getter safely builds the returned object from a validated
response shape instead of spreading entry.response directly; keep the _cacheHit
and _cacheKey metadata on the returned copy.
In `@saas/lib/db.js`:
- Around line 30-56: The current persistence path in save() and recordRequest()
does a synchronous full-file rewrite on every request, which blocks the event
loop and can drop concurrent updates because multiple calls share the same
module-level _data state. Refactor recordRequest() to avoid calling save()
synchronously per request; instead batch or debounce writes, move I/O off the
hot path with async file handling, or replace the file-based implementation with
the better-sqlite3 backend referenced by the module comments. Keep the existing
save() and recordRequest() entry points so the fix is localized, but ensure
writes are serialized and atomic.
- Around line 22-26: The JSON load in the db initialization currently swallows
parse/read failures in the try/catch around fs.readFileSync and JSON.parse,
which can silently reset persisted analytics data. Update the DB_FILE loading
path to catch the error explicitly, log the failure with enough context to
identify the corrupt analytics state, and avoid silently replacing existing data
unless that fallback is intentional and documented; use the db.js initialization
block and the _data assignment as the fix location.
In `@saas/lib/prompt-optimizer.js`:
- Around line 83-92: The optimizeRequest function currently assumes body is a
valid object before accessing body.messages and body.system, so malformed inputs
like null can throw unexpectedly. Add upfront shape/schema validation at the
start of optimizeRequest, before any field access, and return a clear validation
error when body is missing or not the expected object shape. Keep the fix
localized to optimizeRequest and any existing request validation helper used by
prompt-optimizer.js so external payloads fail fast with a descriptive message.
In `@saas/lib/providers.js`:
- Around line 50-53: Update getModel so it does not silently fall back to the
first model when modelId is unknown; instead, fail fast with a clear error or
return a hard failure path for invalid provider/model combinations. Also add
strict validation in the token-count handling code in this module (the logic
around the pricing/analytics calculations) to reject non-numeric, negative, or
non-integer token inputs before they are used. Keep the behavior explicit by
validating provider/model selection and token inputs at the boundaries, using
getModel and the related token-processing functions as the main touchpoints.
In `@saas/lib/token-counter.js`:
- Around line 15-21: The token counting logic in the messages reducer currently
assumes every entry is an object, so malformed items like null or primitives
will crash when accessing msg.content or msg.role. Update the reducer in
token-counter.js to validate each message item before dereferencing it, and
safely skip or handle non-object entries inside the countTokens/messages
processing path. Keep the fix localized to the reducer and its
msg.content/msg.role access points so the function remains robust against
external input.
In `@saas/public/css/dashboard.css`:
- Line 63: Remove the empty CSS rule for the .chart-card.wide selector in
dashboard.css, since it triggers the block-no-empty lint error and has no
effect. Locate the .chart-card.wide block and delete the entire empty
declaration so only meaningful styles remain.
- Line 17: The global body font declaration in the dashboard stylesheet uses
quoted font-family syntax for Inter, which triggers the font-family-name-quotes
lint rule. Update the body rule to use the unquoted Inter family name while
keeping the existing fallback and other styles intact, and verify the change in
the body selector so the lint error is cleared.
In `@saas/public/css/main.css`:
- Line 18: The body style in main.css uses quoted font-family syntax for Inter,
which violates the configured font-family-name-quotes rule. Update the body rule
to use the unquoted Inter family name while keeping the existing fallback and
other declarations unchanged.
In `@saas/public/dashboard.html`:
- Around line 229-265: The Carbon Impact chart is currently plotting savedCost
as a CO₂ proxy in the dashboard timeseries rendering, so update the data flow to
use actual CO₂ if available. Adjust getTimeSeries and the /api/timeseries
payload to include a savedCo2 field, then update the chart logic in the
dashboard rendering code (the labels, tokenData, and co2Chart setup) to plot
that value; if CO₂ is not being tracked, rename the chart/card to cost saved
instead of implying carbon impact.
- Around line 205-219: The dashboard is interpolating untrusted request data
into innerHTML without escaping, which creates an XSS risk. Update the rendering
logic in the table builder that uses tbody.innerHTML so row.provider, row.model,
row.orgId, and each entry in row.techniques are escaped before insertion, or
switch to creating DOM nodes with textContent. Also apply the same escaping
helper in loadTeams() for the org id key interpolation so all dynamic values
rendered in the dashboard are sanitized consistently.
In `@saas/public/index.html`:
- Line 291: The carbon-equivalence math currently hardcodes the same divisors,
so replace the `/ 0.3` and `/ 10 * 100` literals with named constants shared by
the relevant page logic. Update the code in the `impact-compare` calculation to
use those constants, and make the same change in the matching dashboard
calculation so both pages rely on one consistent source of truth. Use clear
symbol names for the constants and keep the math in sync wherever the
equivalence values are computed.
- Around line 281-296: The loadLiveStats polling logic currently swallows all
fetch or JSON parsing failures with an empty catch, which hides persistent
issues. Update the catch in loadLiveStats to handle errors explicitly by logging
at least a debug-level message with the error details, while keeping the polling
behavior resilient. Use the loadLiveStats function and its fetch('/api/stats') /
r.json() path to place the logging where failures occur.
In `@saas/routes/api.js`:
- Around line 17-20: The /recent handler in router.get and the similar
time-series route should not pass NaN through when req.query.n is non-numeric.
Parse the query param into a number, fall back to a safe default when parsing
fails, and then clamp it to the allowed maximum before calling db.getRecent or
db.getTimeSeries; make sure the value is validated in the route handler itself
so invalid input cannot bypass bounds.
In `@saas/routes/proxy.js`:
- Around line 94-113: The semantic cache lookup in the proxy flow is missing org
isolation, so cached chat responses can be shared across tenants. Update the
cache keying used by the proxy route’s cache read/write paths (the
`cache.get(...)` and corresponding `cache.set(...)` call sites in
`saas/routes/proxy.js`) to include `orgId` alongside provider, model, and
messages so each tenant only sees its own cached completions and savings
metadata.
- Around line 171-173: The proxy error handler in the catch block is leaking
internal details by returning err.message to clients. Update the response in the
proxy route to use a generic client-facing error message only, and keep the full
error detail in server-side logging within the same error path. Use the existing
catch (err) logic around the proxy handler in proxy.js and ensure no upstream
hostnames, connection details, or other internals are included in
res.status(502).json.
- Around line 27-63: The forwardRequest helper currently has no upstream
timeout, so a stalled provider can leave the promise unresolved and hold sockets
open. Update forwardRequest in proxy.js to set a timeout on the lib.request call
(or equivalent via request timeout handling), and ensure timed-out requests are
aborted and reject with an error so callers do not hang indefinitely.
- Around line 66-77: The URL rewriting in buildUpstreamUrl is stripping the
wrong prefix, so the provider segment is still being forwarded to the upstream
service. Update the logic in buildUpstreamUrl to remove the leading provider
segment from originalPath instead of matching /proxy/<provider>, and make sure
the provider value is escaped before inserting it into the RegExp so special
characters cannot break the pattern.
In `@saas/server.js`:
- Around line 3-13: The server currently loads environment variables in
server.js but never validates that required provider secrets/config are present
before starting. Add a startup validation step near the app initialization in
server.js that checks the necessary env vars and throws immediately if any are
missing, so the process fails fast instead of surfacing an opaque upstream error
later. Use the existing startup flow around the dotenv config and app/PORT setup
to place this check clearly before routes or server startup logic.
- Around line 36-39: The startup messages in the server entrypoint still use
console.log, which violates the production logging guideline. Replace those
console.log calls in the server bootstrapping code with the project’s proper
logger, keeping the same startup information output through the logger API
instead of direct console usage.
- Around line 15-24: The server setup currently allows unrestricted cross-origin
access and exposes `/api` and `/proxy` without abuse protection. Update the CORS
middleware in `server.js` to use an explicit origin allowlist instead of the
default permissive setup, and add rate limiting middleware (for example, with
express-rate-limit) before mounting `apiRoutes` and `proxyRoutes`. Make sure the
limiter is applied to the public endpoints handled by those route groups and
keep the middleware order so the restrictions run before the routes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d9f1354c-382d-40d4-9945-59edc6cb01ad
⛔ Files ignored due to path filters (1)
saas/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
saas/.env.examplesaas/README.mdsaas/data/analytics.jsonsaas/lib/cache-manager.jssaas/lib/db.jssaas/lib/prompt-optimizer.jssaas/lib/providers.jssaas/lib/token-counter.jssaas/package.jsonsaas/public/css/dashboard.csssaas/public/css/main.csssaas/public/dashboard.htmlsaas/public/index.htmlsaas/routes/api.jssaas/routes/proxy.jssaas/server.js
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx,html,php,java,cs,rb,go}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Implement XSS prevention by sanitizing HTML output
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.jssaas/public/index.htmlsaas/public/dashboard.html
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
saas/lib/providers.jssaas/package.jsonsaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/data/analytics.jsonsaas/lib/db.jssaas/server.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)
**/*.{ts,tsx,js,jsx}: Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation
Use async/await with try-catch for error handling in TypeScript/JavaScript
Use Zod for schema-based input validation in TypeScript/JavaScript
No console.log statements in production code; use proper logging libraries instead
**/*.{ts,tsx,js,jsx}: Auto-format JavaScript/TypeScript files using Prettier after edit
Warn aboutconsole.logstatements in edited files
Check all modified files forconsole.logstatements before session ends
**/*.{ts,tsx,js,jsx}: Use the ApiResponse interface pattern with generic type parameter:interface ApiResponse<T> { success: boolean; data?: T; error?: string; meta?: { total: number; page: number; limit: number; } }
Implement custom React hooks following the pattern: export a named function with use prefix, generic type parameters, and proper useEffect cleanup for side effects
**/*.{ts,tsx,js,jsx}: Never hardcode secrets; always use environment variables for sensitive credentials like API keys
Throw an error when required environment variables are not configured to fail fast and ensure security prerequisites are metUse Playwright as the E2E testing framework for critical user flows in TypeScript/JavaScript
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Always create new objects and never mutate in place; return new copies instead
Keep files between 200–400 lines typical, with a maximum of 800 lines
Extract helpers when a file exceeds 200 lines
Handle errors explicitly at every level; never swallow errors silently
Validate all user input before processing; use schema-based validation where available
Never trust external data (API responses, file content, query params); always validate
All user inputs must be validated and sanitized
Error messages must be scrubbed of sensitive internals
Use readable, well-named identifiers in all code
Keep functions under 50 lines
Keep files under 800 lines
Avoid nesting deeper than 4 levels
Implement comprehensive error handling in all code
Do not hardcode values; use constants or environment configuration instead
Do not use in-place mutation; always return new objects or state
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx,json,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not hardcode secrets, API keys, passwords, or tokens
Files:
saas/lib/providers.jssaas/package.jsonsaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/data/analytics.jsonsaas/lib/db.jssaas/server.js
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use parameterized queries for all database writes (no string interpolation)
Auth/authz must be checked server-side for every sensitive path
Rate limiting must be applied to all public endpoints
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
HTML output must be sanitized where applicable
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Required environment variables must be validated at startup
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt,cpp,c,fs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt,cpp,c,fs}: Write tests before implementation using TDD workflow: write failing test (RED), implement minimal code (GREEN), then refactor (IMPROVE)
Keep functions small (<50 lines) and files focused (<800 lines, typical 200-400 lines)
Avoid deep nesting (>4 levels)
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt}: Never mutate existing objects; always create new objects with changes applied (Immutability requirement)
Handle errors at every level; provide user-friendly messages in UI code and detailed context in server-side logs
Ensure error messages don't leak sensitive data
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.js
**/*.{jsx,tsx,html,js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Sanitize HTML output to prevent XSS vulnerabilities
Files:
saas/lib/providers.jssaas/lib/token-counter.jssaas/lib/prompt-optimizer.jssaas/lib/cache-manager.jssaas/routes/api.jssaas/routes/proxy.jssaas/lib/db.jssaas/server.jssaas/public/index.htmlsaas/public/dashboard.html
**/{index,main,app,server,startup,init}.{js,ts,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Validate that required secrets are present at application startup
Files:
saas/server.js
🪛 ast-grep (0.44.0)
saas/lib/cache-manager.js
[warning] 57-57: Avoid using the initial state variable in setState
Context: setInterval(evictExpired, 5 * 60 * 1000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
saas/routes/proxy.js
[warning] 74-74: Detects non-literal values in regular expressions
Context: new RegExp(^/proxy/${provider})
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
saas/lib/db.js
[warning] 22-22: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(DB_FILE, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 32-32: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(DB_FILE, JSON.stringify(_data, null, 2))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
saas/public/index.html
[warning] 295-295: Avoid using the initial state variable in setState
Context: setInterval(loadLiveStats, 10000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
saas/public/dashboard.html
[warning] 204-218: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: tbody.innerHTML = rows.map(row => {
if (row.savingPct) { totalSavingPct += row.savingPct; count++; }
const techniques = (row.techniques || []).map(t => <span class="tag">${t}</span>).join('');
return <tr> <td><span class="provider-dot ${row.provider}">${row.provider || '-'}</span></td> <td class="mono small">${(row.model || '-').slice(0, 30)}</td> <td class="mono small">${row.orgId || '-'}</td> <td class="num">${fmtNum(row.inputTokens || 0)}</td> <td class="num green-text">-${fmtNum(row.savedTokens || 0)}</td> <td class="num green-text">$${(row.savedCost || 0).toFixed(5)}</td> <td class="num teal-text">${(row.savedCo2 || 0).toFixed(3)}g</td> <td>${techniques || '<span class="tag dim">none</span>'}</td> <td>${row.cacheHit ? '<span class="badge-hit">HIT</span>' : '<span class="badge-miss">MISS</span>'}</td> </tr>;
}).join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 273-280: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: tbody.innerHTML = rows.map(([id, d]) => `
${id}
${fmtNum(d.requests)}
${fmtNum(d.savedTokens)}
$${d.savedCost.toFixed(4)}
${d.savedCo2.toFixed(3)}g
${d.cacheHits}
(inner-outer-html)
[warning] 307-307: Avoid using the initial state variable in setState
Context: setInterval(loadStats, 5000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 308-308: Avoid using the initial state variable in setState
Context: setInterval(loadRecent, 10000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🪛 dotenv-linter (4.0.0)
saas/.env.example
[warning] 5-5: [UnorderedKey] The ANTHROPIC_API_KEY key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 6-6: [UnorderedKey] The GOOGLE_API_KEY key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 7-7: [UnorderedKey] The AWS_ACCESS_KEY_ID key should go before the GOOGLE_API_KEY key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The AWS_SECRET_ACCESS_KEY key should go before the GOOGLE_API_KEY key
(UnorderedKey)
[warning] 11-11: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 12-12: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🪛 HTMLHint (1.9.2)
saas/public/index.html
[warning] 113-113: The type attribute must be present on elements.
(button-type-require)
[warning] 259-259: The type attribute must be present on
elements.(button-type-require)
[warning] 258-258: No matching [ label ] tag found.
(input-requires-label)
saas/public/dashboard.html
[warning] 101-101: The type attribute must be present on elements.
(button-type-require)
[warning] 102-102: The type attribute must be present on
elements.(button-type-require)
[warning] 103-103: The type attribute must be present on
elements.(button-type-require)
[warning] 118-118: The type attribute must be present on
elements.(button-type-require)
[warning] 150-150: The type attribute must be present on
elements.(button-type-require)
🪛 OpenGrep (1.23.0)
saas/data/analytics.json
[ERROR] 164-164: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 263-263: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 410-410: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 460-460: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 1557-1557: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 1688-1688: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 1990-1990: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 2007-2007: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
🪛 Stylelint (17.13.0)
saas/public/css/dashboard.css
[error] 63-63: Empty block (block-no-empty)
(block-no-empty)
[error] 17-17: Expected no quotes around "Inter" (font-family-name-quotes)
(font-family-name-quotes)
saas/public/css/main.css
[error] 18-18: Expected no quotes around "Inter" (font-family-name-quotes)
(font-family-name-quotes)
🔇 Additional comments (4)
saas/lib/db.js (1)
22-22: 🔒 Security & Privacy | 💤 Low valueStatic-analysis path-traversal warnings here are false positives.
DB_FILEis derived solely from__dirnameand fixed literals; no request/user input reaches the path. No action needed.Also applies to: 32-32
saas/data/analytics.json (1)
1-2010: 🔒 Security & Privacy | 💤 Low valueStatic-analysis PII (credit-card) hits are false positives.
The flagged numeric values are token counts, timestamps, and aggregate totals — not PANs. No action needed.
saas/README.md (1)
1-79: LGTM!saas/public/index.html (1)
274-274: 📐 Maintainability & Code Quality | 💤 Low valueVerify the GitHub footer link.
https://github.com/abhijeetbt/ecclooks like a placeholder/personal repo rather than the product repo. Confirm it points where you intend before shipping a public marketing page.
| return null; | ||
| } | ||
| entry.hits++; | ||
| return { ...entry.response, _cacheHit: true, _cacheKey: key }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Cache currently leaks mutable references and has an unsafe spread on reads.
Line 33 stores response by reference; later mutations outside the cache alter cached state. Line 27 spreads entry.response directly, which can fail for null or non-plain objects.
Proposed fix
function get(provider, model, messages) {
@@
entry.hits++;
- return { ...entry.response, _cacheHit: true, _cacheKey: key };
+ const payload = (entry.response && typeof entry.response === 'object')
+ ? structuredClone(entry.response)
+ : { value: entry.response };
+ return { ...payload, _cacheHit: true, _cacheKey: key };
}
function set(provider, model, messages, response, ttlMs = DEFAULT_TTL_MS) {
@@
cache.set(key, {
- response,
+ response: structuredClone(response),
hits: 0,As per coding guidelines, “Always create new objects, never mutate existing ones. Use immutable patterns…” and “Never trust external data.”
Also applies to: 33-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/lib/cache-manager.js` at line 27, In the cache manager read/write path,
the cached response is being stored and returned by reference, which allows
outside mutations to leak into cache state, and the read path is spreading
entry.response without guarding for null or non-plain values. Update the
cache-manager logic around the response storage and retrieval helpers so cached
entries are cloned into new objects when written, and the getter safely builds
the returned object from a validated response shape instead of spreading
entry.response directly; keep the _cacheHit and _cacheKey metadata on the
returned copy.
Source: Coding guidelines
| try { | ||
| _data = JSON.parse(fs.readFileSync(DB_FILE, 'utf8')); | ||
| } catch { | ||
| _data = { requests: [], orgs: {}, totals: { requests: 0, inputTokens: 0, outputTokens: 0, savedTokens: 0, savedCost: 0, savedCo2: 0, cacheHits: 0 } }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Silent JSON parse failure discards persisted data without surfacing the error.
A corrupt analytics.json is caught and replaced with empty state, silently wiping all prior analytics with no log. This swallows the error and masks file corruption.
🛠️ Log and avoid silent data loss
try {
_data = JSON.parse(fs.readFileSync(DB_FILE, 'utf8'));
- } catch {
+ } catch (err) {
+ console.error(`Failed to parse ${DB_FILE}; refusing to overwrite. ${err.message}`);
_data = { requests: [], orgs: {}, totals: { requests: 0, inputTokens: 0, outputTokens: 0, savedTokens: 0, savedCost: 0, savedCo2: 0, cacheHits: 0 } };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| _data = JSON.parse(fs.readFileSync(DB_FILE, 'utf8')); | |
| } catch { | |
| _data = { requests: [], orgs: {}, totals: { requests: 0, inputTokens: 0, outputTokens: 0, savedTokens: 0, savedCost: 0, savedCo2: 0, cacheHits: 0 } }; | |
| } | |
| try { | |
| _data = JSON.parse(fs.readFileSync(DB_FILE, 'utf8')); | |
| } catch (err) { | |
| console.error(`Failed to parse ${DB_FILE}; refusing to overwrite. ${err.message}`); | |
| _data = { requests: [], orgs: {}, totals: { requests: 0, inputTokens: 0, outputTokens: 0, savedTokens: 0, savedCost: 0, savedCo2: 0, cacheHits: 0 } }; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 22-22: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(DB_FILE, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/lib/db.js` around lines 22 - 26, The JSON load in the db initialization
currently swallows parse/read failures in the try/catch around fs.readFileSync
and JSON.parse, which can silently reset persisted analytics data. Update the
DB_FILE loading path to catch the error explicitly, log the failure with enough
context to identify the corrupt analytics state, and avoid silently replacing
existing data unless that fallback is intentional and documented; use the db.js
initialization block and the _data assignment as the fix location.
Source: Coding guidelines
| function save() { | ||
| // Keep only last 10k requests in file | ||
| if (_data.requests.length > 10000) _data.requests = _data.requests.slice(-10000); | ||
| fs.writeFileSync(DB_FILE, JSON.stringify(_data, null, 2)); | ||
| } | ||
|
|
||
| function recordRequest(record) { | ||
| const db = load(); | ||
| db.requests.push({ ...record, ts: Date.now() }); | ||
| db.totals.requests++; | ||
| db.totals.inputTokens += record.inputTokens || 0; | ||
| db.totals.outputTokens += record.outputTokens || 0; | ||
| db.totals.savedTokens += record.savedTokens || 0; | ||
| db.totals.savedCost += record.savedCost || 0; | ||
| db.totals.savedCo2 += record.savedCo2 || 0; | ||
| if (record.cacheHit) db.totals.cacheHits++; | ||
|
|
||
| const orgId = record.orgId || 'default'; | ||
| if (!db.orgs[orgId]) db.orgs[orgId] = { requests: 0, savedTokens: 0, savedCost: 0, savedCo2: 0, cacheHits: 0 }; | ||
| db.orgs[orgId].requests++; | ||
| db.orgs[orgId].savedTokens += record.savedTokens || 0; | ||
| db.orgs[orgId].savedCost += record.savedCost || 0; | ||
| db.orgs[orgId].savedCo2 += record.savedCo2 || 0; | ||
| if (record.cacheHit) db.orgs[orgId].cacheHits++; | ||
|
|
||
| save(); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Synchronous full-file rewrite on every request will block the event loop and lose concurrent writes.
recordRequest calls save() synchronously per request, and save() re-serializes and writeFileSyncs the entire dataset (up to 10k records) on each call. On the Express request thread this blocks the event loop, and concurrent requests interleave read-modify-write on the shared module-level _data, so updates can be lost and the file can be left partially consistent.
Consider batching/debouncing writes, using async I/O off the request path, or switching to the better-sqlite3 backend the header comment alludes to.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 32-32: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(DB_FILE, JSON.stringify(_data, null, 2))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/lib/db.js` around lines 30 - 56, The current persistence path in save()
and recordRequest() does a synchronous full-file rewrite on every request, which
blocks the event loop and can drop concurrent updates because multiple calls
share the same module-level _data state. Refactor recordRequest() to avoid
calling save() synchronously per request; instead batch or debounce writes, move
I/O off the hot path with async file handling, or replace the file-based
implementation with the better-sqlite3 backend referenced by the module
comments. Keep the existing save() and recordRequest() entry points so the fix
is localized, but ensure writes are serialized and atomic.
| function optimizeRequest(body, options = {}) { | ||
| const { | ||
| aggressive = false, | ||
| maxUserTokens = 4000, | ||
| } = options; | ||
|
|
||
| const techniques = []; | ||
| let messages = body.messages ? [...body.messages] : []; | ||
| let systemPrompt = body.system || ''; | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate optimizer input shape before field access.
optimizeRequest assumes body is an object; malformed payloads (e.g., null) will throw before optimization begins. Add upfront schema/shape validation and fail with a clear error.
As per coding guidelines, “Use schema-based validation where available,” “Fail fast with clear error messages when validation fails,” and “Never trust external data.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/lib/prompt-optimizer.js` around lines 83 - 92, The optimizeRequest
function currently assumes body is a valid object before accessing body.messages
and body.system, so malformed inputs like null can throw unexpectedly. Add
upfront shape/schema validation at the start of optimizeRequest, before any
field access, and return a clear validation error when body is missing or not
the expected object shape. Keep the fix localized to optimizeRequest and any
existing request validation helper used by prompt-optimizer.js so external
payloads fail fast with a descriptive message.
Source: Coding guidelines
| function getModel(provider, modelId) { | ||
| const p = PROVIDERS[provider]; | ||
| if (!p) return null; | ||
| return p.models[modelId] || p.models[Object.keys(p.models)[0]] || null; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Fail fast for unknown models and invalid token counts.
Line 53 silently substitutes the first model when modelId is invalid, and Lines 56-61 accept unvalidated token inputs. This can misprice requests and corrupt analytics/CO₂ reporting.
Proposed fix
function getModel(provider, modelId) {
const p = PROVIDERS[provider];
if (!p) return null;
- return p.models[modelId] || p.models[Object.keys(p.models)[0]] || null;
+ if (!modelId || !p.models[modelId]) return null;
+ return p.models[modelId];
}
function calculateCost(provider, modelId, inputTokens, outputTokens) {
const model = getModel(provider, modelId);
- if (!model) return { cost: 0, co2Grams: 0 };
+ if (!model) throw new Error(`Unknown provider/model: ${provider}/${modelId}`);
+ if (!Number.isFinite(inputTokens) || !Number.isFinite(outputTokens) || inputTokens < 0 || outputTokens < 0) {
+ throw new Error('inputTokens/outputTokens must be non-negative finite numbers');
+ }
const cost = (inputTokens / 1_000_000) * model.inputCost + (outputTokens / 1_000_000) * model.outputCost;
const co2Grams = (inputTokens / 1_000_000) * model.co2Per1MInput + (outputTokens / 1_000_000) * model.co2Per1MOutput;
return { cost: parseFloat(cost.toFixed(6)), co2Grams: parseFloat(co2Grams.toFixed(4)) };
}As per coding guidelines, “Always validate all user input before processing at system boundaries,” “Never trust external data,” and “Fail fast with clear error messages when validation fails.”
Also applies to: 56-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/lib/providers.js` around lines 50 - 53, Update getModel so it does not
silently fall back to the first model when modelId is unknown; instead, fail
fast with a clear error or return a hard failure path for invalid provider/model
combinations. Also add strict validation in the token-count handling code in
this module (the logic around the pricing/analytics calculations) to reject
non-numeric, negative, or non-integer token inputs before they are used. Keep
the behavior explicit by validating provider/model selection and token inputs at
the boundaries, using getModel and the related token-processing functions as the
main touchpoints.
Source: Coding guidelines
| // ── Cache check ──────────────────────────────────────────────────────── | ||
| if (isChat && !noCache) { | ||
| const cached = cache.get(provider, model, body.messages); | ||
| if (cached) { | ||
| cacheHit = true; | ||
| const { cost: savedCost, co2Grams: savedCo2 } = calculateCost( | ||
| provider, model, | ||
| cached.usage?.prompt_tokens || cached.usage?.input_tokens || 0, | ||
| cached.usage?.completion_tokens || cached.usage?.output_tokens || 0 | ||
| ); | ||
| db.recordRequest({ | ||
| orgId, provider, model, cacheHit: true, | ||
| inputTokens: 0, outputTokens: 0, | ||
| savedTokens: cached.usage?.prompt_tokens || 0, | ||
| savedCost, savedCo2, techniques: ['cache-hit'], | ||
| savingPct: 100, | ||
| }); | ||
| return res.json({ ...cached, _tokenguard: { cacheHit: true, savedCost, savedCo2Grams: savedCo2 } }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Semantic cache is not scoped per org — one tenant can receive another tenant's cached response.
cache.get(provider, model, body.messages) and cache.set(...) are keyed only by provider/model/messages, with no orgId. Two orgs sending identical messages share cache entries, leaking one org's completion (and savings attribution) to another. Include orgId in the cache key.
Also applies to: 142-144
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/routes/proxy.js` around lines 94 - 113, The semantic cache lookup in the
proxy flow is missing org isolation, so cached chat responses can be shared
across tenants. Update the cache keying used by the proxy route’s cache
read/write paths (the `cache.get(...)` and corresponding `cache.set(...)` call
sites in `saas/routes/proxy.js`) to include `orgId` alongside provider, model,
and messages so each tenant only sees its own cached completions and savings
metadata.
| } catch (err) { | ||
| res.status(502).json({ error: 'TokenGuard proxy error', detail: err.message }); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Error response echoes raw err.message, risking internal detail leakage.
Returning detail: err.message can surface upstream hostnames, stack/connection internals to clients. Log full detail server-side; return a generic message.
🛠️ Scrub client-facing error
} catch (err) {
- res.status(502).json({ error: 'TokenGuard proxy error', detail: err.message });
+ console.error('TokenGuard proxy error:', err);
+ res.status(502).json({ error: 'TokenGuard proxy error' });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (err) { | |
| res.status(502).json({ error: 'TokenGuard proxy error', detail: err.message }); | |
| } | |
| } catch (err) { | |
| console.error('TokenGuard proxy error:', err); | |
| res.status(502).json({ error: 'TokenGuard proxy error' }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/routes/proxy.js` around lines 171 - 173, The proxy error handler in the
catch block is leaking internal details by returning err.message to clients.
Update the response in the proxy route to use a generic client-facing error
message only, and keep the full error detail in server-side logging within the
same error path. Use the existing catch (err) logic around the proxy handler in
proxy.js and ensure no upstream hostnames, connection details, or other
internals are included in res.status(502).json.
Source: Coding guidelines
| require('dotenv').config(); | ||
| const express = require('express'); | ||
| const cors = require('cors'); | ||
| const morgan = require('morgan'); | ||
| const path = require('path'); | ||
|
|
||
| const apiRoutes = require('./routes/api'); | ||
| const proxyRoutes = require('./routes/proxy'); | ||
|
|
||
| const app = express(); | ||
| const PORT = process.env.PORT || 3000; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate required secrets/config at startup; fail fast if missing.
The server loads dotenv but never verifies that required provider credentials/config are present before serving. The README instructs users to add API keys to .env, yet a missing key only surfaces later as an opaque upstream failure.
Add a startup check that throws when required env vars are absent. As per coding guidelines: "Validate that required secrets are present at application startup" and "Throw an error when required environment variables are not configured to fail fast".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/server.js` around lines 3 - 13, The server currently loads environment
variables in server.js but never validates that required provider secrets/config
are present before starting. Add a startup validation step near the app
initialization in server.js that checks the necessary env vars and throws
immediately if any are missing, so the process fails fast instead of surfacing
an opaque upstream error later. Use the existing startup flow around the dotenv
config and app/PORT setup to place this check clearly before routes or server
startup logic.
Source: Coding guidelines
| app.use(cors()); | ||
| app.use(morgan('[:date[clf]] :method :url :status :response-time ms')); | ||
| app.use(express.json({ limit: '10mb' })); | ||
| app.use(express.static(path.join(__dirname, 'public'))); | ||
|
|
||
| // API routes | ||
| app.use('/api', apiRoutes); | ||
|
|
||
| // Proxy routes — e.g. POST /proxy/openai/v1/chat/completions | ||
| app.use('/proxy', proxyRoutes); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no rate-limiting middleware is wired anywhere in saas/
fd -t f -e js . saas | xargs rg -n -i 'rate.?limit|express-rate-limit|slow-down' || echo "No rate limiting found"Repository: affaan-m/ECC
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby route wiring.
git ls-files saas/server.js saas | cat
wc -l saas/server.js
cat -n saas/server.js | sed -n '1,120p'
# Check for any explicit CORS configuration or rate limiting elsewhere in saas/.
rg -n -i --glob 'saas/**/*.js' 'cors\(|express-rate-limit|rate.?limit|slow-down|app\.use\(/proxy|app\.use\(/api' saasRepository: affaan-m/ECC
Length of output: 2123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby route wiring.
git ls-files saas/server.js saas | cat
wc -l saas/server.js
cat -n saas/server.js | sed -n '1,120p'
# Check for any explicit CORS configuration or rate limiting elsewhere in saas/.
rg -n -i --glob 'saas/**/*.js' 'cors\(|express-rate-limit|rate.?limit|slow-down|app\.use\(/proxy|app\.use\(/api' saasRepository: affaan-m/ECC
Length of output: 2123
Restrict CORS and add rate limiting on public endpoints saas/server.js:15-24 app.use(cors()) allows every origin, and there’s still no rate limiting on /api or /proxy. Add an origin allowlist and express-rate-limit (or equivalent) before those routes to reduce abuse and upstream cost.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/server.js` around lines 15 - 24, The server setup currently allows
unrestricted cross-origin access and exposes `/api` and `/proxy` without abuse
protection. Update the CORS middleware in `server.js` to use an explicit origin
allowlist instead of the default permissive setup, and add rate limiting
middleware (for example, with express-rate-limit) before mounting `apiRoutes`
and `proxyRoutes`. Make sure the limiter is applied to the public endpoints
handled by those route groups and keep the middleware order so the restrictions
run before the routes.
Source: Coding guidelines
| console.log(`TokenGuard running on http://localhost:${PORT}`); | ||
| console.log(` Dashboard: http://localhost:${PORT}/dashboard`); | ||
| console.log(` API: http://localhost:${PORT}/api/stats`); | ||
| console.log(` Proxy: http://localhost:${PORT}/proxy/<provider>/...`); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Replace console.log with a proper logger.
Per project rules, production code should use a logging library rather than console.log.
As per coding guidelines: "No console.log statements in production code; use proper logging libraries instead".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@saas/server.js` around lines 36 - 39, The startup messages in the server
entrypoint still use console.log, which violates the production logging
guideline. Replace those console.log calls in the server bootstrapping code with
the project’s proper logger, keeping the same startup information output through
the logger API instead of direct console usage.
Source: Coding guidelines
daltino
left a comment
daltino
left a comment
There was a problem hiding this comment.
This PR appears to align with the project's objectives and guidelines, introducing a TokenGuard feature to optimize API usage. The .env.example file is well-commented, and the provided README.md offers a clear Quick Start guide and integration examples for OpenAI and Anthropic. The inclusion of a lightweight JSON-based storage (db.js) and a locally cached system (cache-manager.js) is a practical choice for this stage, though a note about scaling these for production is included. The PR follows the structure outlined in CONTRIBUTING.md and integrates seamlessly with the project.
3 participants
What Changed
Why This Change
Testing Done
node tests/run-all.js)Type of Change
fix:Bug fixfeat:New featurerefactor:Code refactoringdocs:Documentationtest:Testschore:Maintenance/toolingci:CI/CD changesSecurity & Quality Checklist
Documentation