Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/gateway/src/routes/billing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { writePool as pool } from "../lib/db";
import { authMiddleware } from "../lib/auth";
import { writeAuditLog } from "../lib/audit";
import { apiRateLimiter } from "../middleware/rateLimiting";

const RABBITMQ_URL = process.env.RABBITMQ_URL || "amqp://grainguard:grainguard@rabbitmq:5672/grainguard";
const STRIPE_BILLING_QUEUE = "grainguard.stripe.billing";
Expand Down Expand Up @@ -62,6 +63,7 @@
billingRouter.get(
"/billing/subscription",
authMiddleware,
apiRateLimiter,

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a database access
, but is not rate-limited.

Copilot Autofix

AI about 1 month ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

async (req: Request, res: Response) => {
const { rows } = await pool.query(
`SELECT tb.stripe_customer_id,
Expand Down Expand Up @@ -121,6 +123,7 @@
billingRouter.post(
"/billing/checkout",
authMiddleware,
apiRateLimiter,

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a database access
, but is not rate-limited.

Copilot Autofix

AI about 1 month ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

async (req: Request, res: Response) => {
const { plan } = req.body as { plan: string };

Expand Down Expand Up @@ -188,6 +191,7 @@
billingRouter.post(
"/billing/portal",
authMiddleware,
apiRateLimiter,

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a database access
, but is not rate-limited.

Copilot Autofix

AI about 1 month ago

In general, to fix missing rate limiting on an expensive route, introduce a rate-limiting middleware (or reuse an existing one) and apply it to that route so that the number of accepted requests per client (or globally) is capped over a time window. This protects backing services (database, queues, external APIs) from overload due to bursts of requests.

In this file, the best fix with minimal functional change is to apply the existing apiRateLimiter middleware (already imported and used on /billing/checkout and /billing/portal) to the stripeWebhookHandler route. Since stripeWebhookHandler is currently exported as a bare function, the most consistent change—while staying within the snippet—is to wrap it in an Express router post definition that includes apiRateLimiter. However, the surrounding router wiring isn’t shown, so the smallest safe change is to define and export a new Express router that mounts the webhook handler with apiRateLimiter, and adjust the handler to be an Express-style middleware in that router. Within this snippet, we can convert stripeWebhookHandler from a standalone exported function to a billingRouter.post("/billing/webhook", authMiddleware?, apiRateLimiter, handler) style definition, mirroring the pattern used for the other routes. Because we must not assume changes in other files, we keep the handler logic intact and just add apiRateLimiter into the middleware chain for the webhook endpoint.

Concretely:

  • Locate the export async function stripeWebhookHandler definition near the end of apps/gateway/src/routes/billing.ts.
  • Replace that exported function with a billingRouter.post("/billing/webhook", apiRateLimiter, async (req, res) => { ... }), reusing the existing handler body unchanged.
  • If billingRouter is already defined earlier in the file (as implied by billingRouter.post("/billing/checkout", ...) and billingRouter.post("/billing/portal", ...)), no new definitions or imports are needed.
  • This ensures that every incoming webhook request passes through apiRateLimiter before executing the expensive RabbitMQ publishing logic.
Suggested changeset 1
apps/gateway/src/routes/billing.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/gateway/src/routes/billing.ts b/apps/gateway/src/routes/billing.ts
--- a/apps/gateway/src/routes/billing.ts
+++ b/apps/gateway/src/routes/billing.ts
@@ -220,31 +220,37 @@
   }
 );
 
-export async function stripeWebhookHandler(req: Request, res: Response) {
-  const sig = req.headers["stripe-signature"] as string;
+billingRouter.post(
+  "/billing/webhook",
+  apiRateLimiter,
+  async (req: Request, res: Response) => {
+    const sig = req.headers["stripe-signature"] as string;
 
-  let event: Stripe.Event;
-  try {
-    event = stripe.webhooks.constructEvent(
-      req.body,
-      sig,
-      process.env.STRIPE_WEBHOOK_SECRET!
-    );
-  } catch (err: any) {
-    console.error("[billing] webhook signature failed:", err.message);
-    return res.status(400).send(`Webhook Error: ${err.message}`);
-  }
+    let event: Stripe.Event;
+    try {
+      event = stripe.webhooks.constructEvent(
+        req.body,
+        sig,
+        process.env.STRIPE_WEBHOOK_SECRET!
+      );
+    } catch (err: any) {
+      console.error("[billing] webhook signature failed:", err.message);
+      return res.status(400).send(`Webhook Error: ${err.message}`);
+    }
 
-  try {
-    await publishToStripeQueue({
-      stripeEventId: event.id,
-      stripeEventType: event.type,
-      payload: event.data.object,
-    });
-    console.log(`[billing] queued stripe event ${event.type} id=${event.id}`);
-    return res.json({ received: true });
-  } catch (err) {
-    console.error("[billing] failed to queue stripe event:", err);
-    return res.status(500).json({ error: "queue_publish_failed" });
+    try {
+      await publishToStripeQueue({
+        stripeEventId:  event.id,
+        stripeEventType: event.type,
+        payload:        event.data.object,
+      });
+      console.log(
+        `[billing] queued stripe event ${event.type} id=${event.id}`
+      );
+      return res.json({ received: true });
+    } catch (err) {
+      console.error("[billing] failed to queue stripe event:", err);
+      return res.status(500).json({ error: "queue_publish_failed" });
+    }
   }
-}
+);
EOF
@@ -220,31 +220,37 @@
}
);

export async function stripeWebhookHandler(req: Request, res: Response) {
const sig = req.headers["stripe-signature"] as string;
billingRouter.post(
"/billing/webhook",
apiRateLimiter,
async (req: Request, res: Response) => {
const sig = req.headers["stripe-signature"] as string;

let event: Stripe.Event;
try {
event = stripe.webhooks.constructEvent(
req.body,
sig,
process.env.STRIPE_WEBHOOK_SECRET!
);
} catch (err: any) {
console.error("[billing] webhook signature failed:", err.message);
return res.status(400).send(`Webhook Error: ${err.message}`);
}
let event: Stripe.Event;
try {
event = stripe.webhooks.constructEvent(
req.body,
sig,
process.env.STRIPE_WEBHOOK_SECRET!
);
} catch (err: any) {
console.error("[billing] webhook signature failed:", err.message);
return res.status(400).send(`Webhook Error: ${err.message}`);
}

try {
await publishToStripeQueue({
stripeEventId: event.id,
stripeEventType: event.type,
payload: event.data.object,
});
console.log(`[billing] queued stripe event ${event.type} id=${event.id}`);
return res.json({ received: true });
} catch (err) {
console.error("[billing] failed to queue stripe event:", err);
return res.status(500).json({ error: "queue_publish_failed" });
try {
await publishToStripeQueue({
stripeEventId: event.id,
stripeEventType: event.type,
payload: event.data.object,
});
console.log(
`[billing] queued stripe event ${event.type} id=${event.id}`
);
return res.json({ received: true });
} catch (err) {
console.error("[billing] failed to queue stripe event:", err);
return res.status(500).json({ error: "queue_publish_failed" });
}
}
}
);
Copilot is powered by AI and may make mistakes. Always verify output.
async (req: Request, res: Response) => {
const { rows } = await pool.query(
`SELECT stripe_customer_id FROM tenant_billing WHERE tenant_id = $1`,
Expand Down
7 changes: 3 additions & 4 deletions apps/gateway/src/services/device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import fs from "fs";
📦 Load Proto
========================================= */

const protoPath = path.resolve(
__dirname,
"../../../../libs/proto/device.proto"
);
const protoPath = fs.existsSync("/app/libs/proto/device.proto")
? "/app/libs/proto/device.proto"
: path.resolve(__dirname, "../../libs/proto/device.proto");
Comment on lines +11 to +13
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fallback path incorrect for local development.

The primary absolute path /app/libs/proto/device.proto correctly resolves inside the Docker container. However, the fallback ../../libs/proto/device.proto assumes a flat structure that doesn't match local development.

When running locally from apps/gateway/dist/services/device.js, ../../libs/ resolves to apps/gateway/libs/, but the proto file lives at the repo root (libs/proto/device.proto), requiring ../../../../libs/proto/device.proto.

If local dev support isn't needed, consider adding a comment. Otherwise:

🔧 Suggested fix to support both environments
-const protoPath = fs.existsSync("/app/libs/proto/device.proto")
-  ? "/app/libs/proto/device.proto"
-  : path.resolve(__dirname, "../../libs/proto/device.proto");
+// Docker: proto at /app/libs/proto/device.proto
+// Local:  proto at repo root, 4 levels up from apps/gateway/dist/services/
+const protoPath = fs.existsSync("/app/libs/proto/device.proto")
+  ? "/app/libs/proto/device.proto"
+  : path.resolve(__dirname, "../../../../libs/proto/device.proto");
📝 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.

Suggested change
const protoPath = fs.existsSync("/app/libs/proto/device.proto")
? "/app/libs/proto/device.proto"
: path.resolve(__dirname, "../../libs/proto/device.proto");
// Docker: proto at /app/libs/proto/device.proto
// Local: proto at repo root, 4 levels up from apps/gateway/dist/services/
const protoPath = fs.existsSync("/app/libs/proto/device.proto")
? "/app/libs/proto/device.proto"
: path.resolve(__dirname, "../../../../libs/proto/device.proto");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/gateway/src/services/device.ts` around lines 11 - 13, The fallback path
for protoPath is wrong for local dev: update the fallback in the protoPath
expression (the ternary using fs.existsSync and __dirname) to point to the
repo-root location (e.g., use path.resolve(__dirname,
"../../../../libs/proto/device.proto") or path.resolve(__dirname, "../../../..",
"libs/proto/device.proto")) so when running apps/gateway/dist/services/device.js
it resolves to libs/proto/device.proto; alternatively, if local development
support is intentionally not required, add a clarifying comment above the
protoPath declaration noting the code only supports the container absolute path.


const packageDefinition = protoLoader.loadSync(protoPath, {
keepCase: true,
Expand Down
2 changes: 1 addition & 1 deletion scripts/load-tests/performance-budget.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function hasGraphqlErrors(response) {
try {
const errors = response.json("errors");
return Array.isArray(errors) && errors.length > 0;
} catch {
} catch (error) {
return true;
}
Comment on lines +85 to 87
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Unused error parameter in catch block.

The error parameter is captured but never used. Consider either:

  1. Logging the error for debugging parse failures during load tests, or
  2. Keeping the ES2019+ optional catch binding syntax (catch {) if the error is not needed
Option 1: Log the error for debugging
-  } catch (error) {
+  } catch (e) {
+    console.warn('GraphQL response parse failed:', e.message);
     return true;
   }
Option 2: Use optional catch binding (cleaner if error is not needed)
-  } catch (error) {
+  } catch {
     return true;
   }
📝 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.

Suggested change
} catch (error) {
return true;
}
} catch {
return true;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/load-tests/performance-budget.js` around lines 85 - 87, The catch
block currently declares catch (error) but never uses it; either remove the
unused binding by changing it to the optional catch binding (catch { return
true; }) or log the thrown error for debugging by keeping catch (error) and
adding a logging statement before returning (e.g., console.error or the existing
logger) so the code becomes catch (error) { console.error('Parse failed in
performance budget check:', error); return true; } — update the catch around the
failing parse/validation in performance-budget.js accordingly.

}
Expand Down
Loading