Skip to content

fix(ci): golangci-lint v2, SQL injection fix, BFF/dashboard lint cleanup #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pahuldeepp merged 9 commits into from
Mar 28, 2026
Merged

fix(ci): golangci-lint v2, SQL injection fix, BFF/dashboard lint cleanup #10

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
89648be
fix(stack): harden e2e auth and tenant-scoped queries
pahuldeepp Mar 28, 2026
0fc10a2
fix(ci): resolve Go Lint SA6002 and CodeQL SQL injection alert
pahuldeepp Mar 28, 2026
203d878
fix(ci): move exclude-dirs to run section for golangci-lint v2
pahuldeepp Mar 28, 2026
8ae97ca
fix(lint): eliminate all explicit any warnings in bff and dashboard
pahuldeepp Mar 28, 2026
486f70e
fix(ci): correct golangci-lint v2 schema and catch type narrowing
pahuldeepp Mar 28, 2026
b3fda45
fix(bff): add toIsoDate helper for unknown-typed row fields
pahuldeepp Mar 28, 2026
5689f8e
fix(lint): suppress expected Go lint false-positives
pahuldeepp Mar 28, 2026
e44aa9d
fix(lint): resolve all remaining golangci-lint errcheck and gosec fai…
pahuldeepp Mar 28, 2026
e31fa48
fix(go): resolve remaining lint failures
pahuldeepp Mar 28, 2026
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
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ jobs:
cache: true

- name: golangci-lint
run: go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.62.0 run --timeout=5m
uses: golangci/golangci-lint-action@v9
with:
version: v2.11.0
args: --timeout=5m

go-test:
name: Go Test
Expand Down
58 changes: 25 additions & 33 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,53 +1,45 @@
version: "2"

run:
timeout: 5m
modules-download-mode: readonly

linters:
default: none
enable:
- errcheck
- govet
- staticcheck
- unused
- gosimple
- ineffassign
- typecheck
- gocritic
- gosec
- prealloc
- bodyclose
- noctx
- exhaustive
Comment on lines +8 to 19

@coderabbitai coderabbitai Bot Mar 28, 2026

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

Consider re-enabling unused and gosimple linters.

Setting default: none and explicitly enabling linters is clean for v2, but unused (dead code detection) and gosimple (code simplifications) provide valuable feedback that was previously enabled. Their removal may allow technical debt to accumulate undetected.

If these were intentionally removed due to noise, consider documenting the rationale in a comment.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.golangci.yml around lines 12 - 23, Re-enable the unused and gosimple
linters by adding "unused" and "gosimple" to the enabled list in the
.golangci.yml (i.e., include them alongside errcheck, govet, staticcheck, etc.),
or if they were intentionally disabled, add a short comment in the file
explaining the rationale and where to revisit this decision; reference the
existing enable block that lists errcheck, govet, staticcheck, ineffassign,
gocritic, gosec, prealloc, bodyclose, noctx, exhaustive to locate where to add
the two linters or the explanatory comment.

disable:
- depguard

linters-settings:
govet:
enable-all: true
disable:
- fieldalignment
gocritic:
enabled-tags:
- diagnostic
- performance
disabled-checks:
- ifElseChain
- exitAfterDefer
- hugeParam
- rangeValCopy
- sloppyReassign
- sprintfQuotedString
gosec:
excludes:
- G104 # unhandled errors in deferred calls (noisy)
- G404 # math/rand is acceptable here for jitter and synthetic test data
exhaustive:
default-signifies-exhaustive: true
settings:
govet:
enable-all: true
disable:
- fieldalignment
gocritic:
enabled-tags:
- diagnostic
- performance
disabled-checks:
- ifElseChain
- exitAfterDefer
- hugeParam
- rangeValCopy
- sloppyReassign
- sprintfQuotedString
gosec:
excludes:
- G104 # unhandled errors in deferred calls (noisy)
- G404 # math/rand is acceptable here for jitter and synthetic test data
exhaustive:
default-signifies-exhaustive: true

issues:
exclude-use-default: true
max-issues-per-linter: 50
max-same-issues: 5
exclude-dirs:
- vendor
- node_modules
- dist
25 changes: 18 additions & 7 deletions apps/bff/src/datasources/elasticsearch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ export const es = new Client({ node: ES_URL });

const DEVICE_INDEX = "grainguard-devices";

interface ESHit {
_source: {
device_id: string;
tenant_id: string;
serial_number: string;
temperature?: number;
humidity?: number;
recorded_at?: string;
status?: string;
};
_score: number;
}

export const search = {
async searchDevices(query: string, tenantId: string, limit = 20) {
try {
Expand All @@ -31,11 +44,9 @@ export const search = {
},
},
};
console.log("[ES] searching:", JSON.stringify(params));
const result = await (es.search as any)(params);
console.log("[ES] hits:", result.hits.total);
const result = await (es.search as (p: typeof params) => Promise<{ hits: { total: unknown; hits: ESHit[] } }>)(params);

return result.hits.hits.map((hit: any) => ({
return result.hits.hits.map((hit: ESHit) => ({
Comment on lines +47 to +49

@coderabbitai coderabbitai Bot Mar 28, 2026

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

🧩 Analysis chain

🌐 Web query:

@elastic/elasticsearch typescript generic search response

💡 Result:

In the @elastic/elasticsearch Node.js client, the generic search response type is SearchResponse<TDocument, TAggregations>, imported from '@elastic/elasticsearch/lib/api/types' or via 'estypes'. Key usage: import { Client } from '@elastic/elasticsearch'; interface MyDocument { title: string; content: string; } const result = await client.search({ index: 'my-index', query: { match_all: {} } }); // result is typed as TransportResult<SearchResponse, unknown> or SearchResponse depending on options The SearchResponse<TDocument = unknown, TAggregations = Record<AggregateName, AggregationsAggregate>> interface includes: - took: number - timed_out: boolean - _shards: ShardStatistics - hits: SearchHitsMetadata - aggregations?: TAggregations - etc. For modern "new types", pass the document type directly to search. Official docs confirm first-class TypeScript support with generics for search responses, using snake_case params. Example from docs: interface Document { character: string; quote: string; } const result = await client.search({ index: 'game-of-thrones', query: { match: { quote: 'winter' } } }); Source code shows: export default async function SearchApi<TDocument = unknown, TAggregations = Record<T.AggregateName, T.AggregationsAggregate>>(...): Promise<T.SearchResponse<TDocument, TAggregations>> Legacy types used explicit SearchResponse but current defaults to unknown with generics.

Citations:

🏁 Script executed:

cat -n apps/bff/src/datasources/elasticsearch.ts | head -60

Repository: pahuldeepp/GrainGuard-

Length of output: 2220

🏁 Script executed:

cat -n apps/bff/src/datasources/elasticsearch.ts | tail -30

Repository: pahuldeepp/GrainGuard-

Length of output: 1275

🏁 Script executed:

rg -l "elasticsearch" apps/bff/src --type ts | head -10

Repository: pahuldeepp/GrainGuard-

Length of output: 133

🏁 Script executed:

rg "searchDevices" apps/bff/src/resolvers.ts -B 3 -A 5

Repository: pahuldeepp/GrainGuard-

Length of output: 648

🏁 Script executed:

rg "BffContext|tenantFilter" apps/bff/src --type ts -B 2 -A 2 | head -50

Repository: pahuldeepp/GrainGuard-

Length of output: 2639

Refactor to use typed generics for better type safety.

The @elastic/elasticsearch client officially supports typed generics for search responses. Replace the manual type cast with:

const result = await es.search<ESHit['_source']>({...params});

This eliminates the unconventional cast and provides first-class TypeScript type safety without manual assertions.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/bff/src/datasources/elasticsearch.ts` around lines 47 - 49, Replace the
manual cast on the es.search call with the client’s generic typing: call
es.search with the generic type parameter for the hit _source (use
ESHit['_source']) instead of casting the function; update the declaration where
you currently do (es.search as (p: typeof params) => Promise<...>)(params) to a
direct es.search<ESHit['_source']>(params) so the returned result has proper
typed hits for the subsequent result.hits.hits.map usage.

deviceId: hit._source.device_id,
tenantId: hit._source.tenant_id,
serialNumber: hit._source.serial_number,
Expand All @@ -45,10 +56,10 @@ export const search = {
status: hit._source.status ?? null,
score: hit._score,
}));
} catch (err: any) {
console.error("[ES] error:", err?.meta?.body?.error || err?.message || err);
} catch (err: unknown) {
const e = err as { meta?: { body?: { error?: unknown } }; message?: string };
console.error("[ES] error:", e?.meta?.body?.error || e?.message || err);
return [];
}
},
};

67 changes: 43 additions & 24 deletions apps/bff/src/datasources/postgres.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,41 @@ const pool = new Pool({
max: 50,
});

type Row = Record<string, unknown>;
type QueryResult = import("pg").QueryResult<Row>;

// Circuit-breaker-wrapped query helper
async function cbQuery(text: string, values?: any[]): Promise<import("pg").QueryResult<any>> {
return postgresCircuitBreaker.execute(() => pool.query(text, values));
async function cbQuery(text: string, values?: unknown[]): Promise<QueryResult> {
return postgresCircuitBreaker.execute(() => pool.query(text, values as unknown[]));
}

// Tenant-scoped query — sets app.current_tenant_id for RLS enforcement
export async function tenantQuery(
tenantId: string,
text: string,
values?: any[]
): Promise<import("pg").QueryResult<any>> {
values?: unknown[]
): Promise<QueryResult> {
return postgresCircuitBreaker.execute(async () => {
const client = await pool.connect();
try {
await client.query('SET LOCAL app.current_tenant_id = $1', [tenantId]);
return await client.query(text, values);
await client.query("BEGIN");
await client.query(
"SELECT set_config('app.current_tenant_id', $1, true)",
[tenantId]
);
const result = await client.query(
text,
values as unknown[] | undefined
) as QueryResult;
await client.query("COMMIT");
return result;
} catch (error) {
try {
await client.query("ROLLBACK");
} catch {
// Release the client even if rollback fails.
}
throw error;
} finally {
client.release();
}
Expand All @@ -52,13 +71,13 @@ export const db = {

async getAllDevices(limit: number = 20) {
const cacheKey = `devices:all:${limit}`;
const cached = await cache.get<any[]>(cacheKey);
const cached = await cache.get<Row[]>(cacheKey);
if (cached) return cached;

const locked = await cache.acquireLock(cacheKey, 5);
if (!locked) {
await new Promise(r => setTimeout(r, 100));
return await cache.get<any[]>(cacheKey) || [];
return await cache.get<Row[]>(cacheKey) || [];
}

try {
Expand Down Expand Up @@ -88,13 +107,13 @@ export const db = {

async getAllTelemetry(limit: number = 20, tenantId?: string) {
const cacheKey = `telemetry:all:${tenantId || "global"}:${limit}`;
const cached = await cache.get<any[]>(cacheKey);
const cached = await cache.get<Row[]>(cacheKey);
if (cached) return cached;

const locked = await cache.acquireLock(cacheKey, 5);
if (!locked) {
await new Promise(r => setTimeout(r, 100));
return await cache.get<any[]>(cacheKey) || [];
return await cache.get<Row[]>(cacheKey) || [];
}

try {
Expand Down Expand Up @@ -148,7 +167,7 @@ export const db = {

async getTelemetryHistory(deviceId: string, limit = 50, tenantId?: string) {
const queryFn = tenantId
? (text: string, values: any[]) => tenantQuery(tenantId, text, values)
? (text: string, values: unknown[]) => tenantQuery(tenantId, text, values)
: cbQuery;
const result = await queryFn(
`SELECT device_id, temperature, humidity, recorded_at
Expand All @@ -158,7 +177,7 @@ export const db = {
LIMIT $2`,
[deviceId, limit]
);
return result.rows.map((r: any) => ({
return result.rows.map((r: Row) => ({
deviceId: r.device_id,
temperature: r.temperature,
humidity: r.humidity,
Expand All @@ -168,13 +187,13 @@ export const db = {

async getAllDevicesWithTelemetry(limit: number = 20, tenantId?: string) {
const cacheKey = `devices:telemetry:${tenantId || "global"}:${limit}`;
const cached = await cache.get<any[]>(cacheKey);
const cached = await cache.get<Row[]>(cacheKey);
if (cached) return cached;

const locked = await cache.acquireLock(cacheKey, 5);
if (!locked) {
await new Promise(r => setTimeout(r, 100));
return await cache.get<any[]>(cacheKey) || [];
return await cache.get<Row[]>(cacheKey) || [];
}

try {
Expand Down Expand Up @@ -222,7 +241,7 @@ export const db = {
}

const fetchLimit = first + 1;
let rows: any[];
let rows: Row[];

if (tenantId && afterTimestamp) {
const result = await cbQuery(
Expand Down Expand Up @@ -270,22 +289,22 @@ export const db = {
"SELECT COUNT(*) FROM device_projections WHERE tenant_id = $1",
[tenantId]
);
totalCount = parseInt(countResult.rows[0].count, 10);
totalCount = parseInt(countResult.rows[0].count as string, 10);
} else {
const countResult = await cbQuery("SELECT COUNT(*) FROM device_projections");
totalCount = parseInt(countResult.rows[0].count, 10);
totalCount = parseInt(countResult.rows[0].count as string, 10);
}

const edges = items.map((row: any) => ({
cursor: Buffer.from(row.created_at.toISOString()).toString("base64"),
const edges = items.map((row: Row) => ({
cursor: Buffer.from((row.created_at as Date).toISOString()).toString("base64"),
node: {
deviceId: row.device_id,
tenantId: row.tenant_id,
serialNumber: row.serial_number,
createdAt: new Date(row.created_at).toISOString(),
createdAt: new Date(row.created_at as string).toISOString(),
temperature: row.temperature ?? null,
humidity: row.humidity ?? null,
recordedAt: row.recorded_at ? new Date(row.recorded_at).toISOString() : null,
recordedAt: row.recorded_at ? new Date(row.recorded_at as string).toISOString() : null,
Comment on lines +298 to +307

@coderabbitai coderabbitai Bot Mar 28, 2026

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

Inconsistent Date handling between cursor and node fields.

Line 299 casts row.created_at as Date and calls .toISOString() directly, while line 304 wraps it in new Date(row.created_at as string). The pg driver returns timestamp columns as JavaScript Date objects, so line 299's approach is correct but line 304 is redundant (and would fail if given an actual Date object).

♻️ Suggested consistency fix 
       cursor: Buffer.from((row.created_at as Date).toISOString()).toString("base64"),
       node: {
         deviceId:     row.device_id,
         tenantId:     row.tenant_id,
         serialNumber: row.serial_number,
-        createdAt:    new Date(row.created_at as string).toISOString(),
+        createdAt:    (row.created_at as Date).toISOString(),
         temperature:  row.temperature ?? null,
         humidity:     row.humidity ?? null,
-        recordedAt:   row.recorded_at ? new Date(row.recorded_at as string).toISOString() : null,
+        recordedAt:   row.recorded_at ? (row.recorded_at as Date).toISOString() : null,
         version:      row.version ?? null,
       },
📝 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 edges = items.map((row: Row) => ({
cursor: Buffer.from((row.created_at as Date).toISOString()).toString("base64"),
node: {
deviceId: row.device_id,
tenantId: row.tenant_id,
serialNumber: row.serial_number,
createdAt: new Date(row.created_at).toISOString(),
createdAt: new Date(row.created_at as string).toISOString(),
temperature: row.temperature ?? null,
humidity: row.humidity ?? null,
recordedAt: row.recorded_at ? new Date(row.recorded_at).toISOString() : null,
recordedAt: row.recorded_at ? new Date(row.recorded_at as string).toISOString() : null,
const edges = items.map((row: Row) => ({
cursor: Buffer.from((row.created_at as Date).toISOString()).toString("base64"),
node: {
deviceId: row.device_id,
tenantId: row.tenant_id,
serialNumber: row.serial_number,
createdAt: (row.created_at as Date).toISOString(),
temperature: row.temperature ?? null,
humidity: row.humidity ?? null,
recordedAt: row.recorded_at ? (row.recorded_at as Date).toISOString() : null,
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/bff/src/datasources/postgres.ts` around lines 298 - 307, The code
constructs edges via items.map and inconsistently handles created_at: the cursor
uses (row.created_at as Date).toISOString() while node.createdAt wraps it in new
Date(...), which is redundant and can fail; update node.createdAt to mirror the
cursor by using (row.created_at as Date).toISOString() and similarly ensure
recordedAt handles row.recorded_at safely by checking if it's a Date then
calling .toISOString() (e.g., recordedAt: row.recorded_at ? (row.recorded_at as
Date).toISOString() : null) so both cursor and node fields consistently treat
timestamps as Date objects.

version: row.version ?? null,
},
}));
Expand All @@ -301,6 +320,7 @@ export const db = {
},
};
},

async createDevice(input: { serialNumber: string; tenantId: string }) {
const result = await cbQuery(
`INSERT INTO device_projections (device_id, tenant_id, serial_number, created_at)
Expand All @@ -312,7 +332,7 @@ export const db = {
deviceId: result.rows[0].device_id,
tenantId: result.rows[0].tenant_id,
serialNumber: result.rows[0].serial_number,
createdAt: new Date(result.rows[0].created_at).toISOString(),
createdAt: new Date(result.rows[0].created_at as string).toISOString(),
};
Comment on lines +335 to 336

@coderabbitai coderabbitai Bot Mar 28, 2026

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

Same inconsistency in createDevice and updateDevice.

These also use new Date(result.rows[0].created_at as string) which is inconsistent with the cursor handling.

Also applies to: 351-352

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/bff/src/datasources/postgres.ts` around lines 335 - 336, In createDevice
and updateDevice the createdAt field is built using new
Date(result.rows[0].created_at as string).toISOString(), which is inconsistent
with how cursors are handled elsewhere; replace that direct Date construction
with the same cursor-decoding helper used by other cursor logic (e.g., the
project function that converts stored cursor timestamps to ISO such as
parseCursorDate/decodeCursor/convertCursorToISO) and pass
result.rows[0].created_at through that helper so createdAt is produced the same
way as other cursor-derived dates.

},

Expand All @@ -328,7 +348,7 @@ export const db = {
deviceId: result.rows[0].device_id,
tenantId: result.rows[0].tenant_id,
serialNumber: result.rows[0].serial_number,
createdAt: new Date(result.rows[0].created_at).toISOString(),
createdAt: new Date(result.rows[0].created_at as string).toISOString(),
};
},

Expand All @@ -340,4 +360,3 @@ export const db = {
},

};

2 changes: 1 addition & 1 deletion apps/bff/src/datasources/redis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const cache = {
return results.map((r: any) => r ? JSON.parse(r) : null);

@coderabbitai coderabbitai Bot Mar 28, 2026

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

Remaining any in getMany callback.

Line 27 still uses (r: any) which is inconsistent with the PR's goal of eliminating explicit any usage. Consider typing the pipeline results properly.

♻️ Proposed fix 
-    return results.map((r: any) => r ? JSON.parse(r) : null);
+    return results.map((r) => (typeof r === "string" ? JSON.parse(r) : null));
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/bff/src/datasources/redis.ts` at line 27, The map callback in getMany
still uses (r: any); change it to a properly typed signature reflecting pipeline
results (e.g., (r: string | null)) and propagate a generic type for parsed
results (e.g., getMany<T> returning (T | null)[]). Update the mapping to use (r:
string | null) => r ? JSON.parse(r) as T : null and consider wrapping JSON.parse
in a try/catch or a safe parser if needed; reference the getMany function and
the results variable (pipeline output) when making the change.

},

async set(key: string, value: any, ttlSeconds: number): Promise<void> {
async set(key: string, value: unknown, ttlSeconds: number): Promise<void> {
await client.set(key, JSON.stringify(value), { EX: ttlSeconds });
},

Expand Down
15 changes: 8 additions & 7 deletions apps/bff/src/lib/circuitBreaker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,17 @@ export class CircuitBreaker {
const result = await fn();
this.onSuccess();
return result;
} catch (err: any) {
} catch (err: unknown) {
// Only count real infrastructure failures (connection loss, timeout).
// Bad SQL (invalid UUID, syntax errors) are app bugs, not Postgres outages.
const e = err as { code?: string; message?: string };
const isInfraFailure =
err?.code === "ECONNREFUSED" ||
err?.code === "ENOTFOUND" ||
err?.code === "ETIMEDOUT" ||
err?.code === "ECONNRESET" ||
err?.message?.includes("Connection terminated") ||
err?.message?.includes("connect ECONNREFUSED");
e.code === "ECONNREFUSED" ||
e.code === "ENOTFOUND" ||
e.code === "ETIMEDOUT" ||
e.code === "ECONNRESET" ||
e.message?.includes("Connection terminated") ||
e.message?.includes("connect ECONNREFUSED");

if (isInfraFailure) this.onFailure();
throw err;
Expand Down
3 changes: 2 additions & 1 deletion apps/bff/src/observability/metrics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import client from "prom-client";
import type { Request, Response } from "express";

const register = new client.Registry();
client.collectDefaultMetrics({ register });
Expand All @@ -19,7 +20,7 @@ export const graphqlOperations = new client.Counter({
});

export function metricsHandler() {
return async (_req: any, res: any) => {
return async (_req: Request, res: Response) => {
res.setHeader("Content-Type", register.contentType);
res.end(await register.metrics());
};
Expand Down
Loading
Loading