feat(snmp): add OID policy enforcement and audit persistence scaffold - #2
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfd99cb85b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SNMP_RETRIES: z.coerce.number().default(2), | ||
| OID_ALLOWLIST: z.string().default('1.3.6.1.2.1'), | ||
| OID_DENYLIST: z.string().default('1.3.6.1.6.3.15'), | ||
| AUDIT_PERSISTENCE_ENABLED: z.coerce.boolean().default(false), |
There was a problem hiding this comment.
Parse audit flag with string-safe boolean semantics
AUDIT_PERSISTENCE_ENABLED is loaded from environment variables as a string, but z.coerce.boolean() uses JavaScript truthiness, so values like 'false' or '0' parse to true. With the shipped .env value AUDIT_PERSISTENCE_ENABLED=false, audit persistence is unintentionally enabled and SNMP requests start writing to Postgres, which can fail requests when DB/audit schema is not available.
Useful? React with 👍 / 👎.
| }); | ||
| }); | ||
|
|
||
| session.close(); |
There was a problem hiding this comment.
Always close SNMP sessions when get() fails
The session is closed only after the awaited promise resolves; any session.get error path (error, empty varbinds, or mapping exception) rejects before reaching session.close(). In repeated timeout/error scenarios this leaks SNMP sessions/sockets and can exhaust resources, so closure should happen in a finally block.
Useful? React with 👍 / 👎.
| data: result, | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof AppError) { |
There was a problem hiding this comment.
Return 4xx for malformed OID input
The route only converts AppError instances to client-facing status codes; other errors are rethrown and become 500s. Since OID format validation currently throws a plain Error, requests with a valid shape but malformed OID (for example invalid-oid plus a full connection) return Internal Server Error instead of a client error, which breaks the API's invalid-payload behavior.
Useful? React with 👍 / 👎.
Motivation
Description
OidPolicyServicewith allowlist/denylist prefix rules and wired it into the SNMP flow after OID normalization and before adapter execution (new filesrc/modules/security/oid-policy.service.ts).AUDIT_PERSISTENCE_ENABLED(new files undersrc/modules/audit/andsrc/shared/database/client.ts).AppError,OidBlockedError,UnsupportedVendorError) and mapped them to HTTP responses in SNMP route handler (src/shared/errors/app-error.ts,src/app/routes/v1/snmp.routes.ts)..env.exampleentries for OID policy and audit flags, and added TypeScript types fornet-snmpand a generic SNMP adapter implementation.tests/unit/*,tests/integration/*).Testing
npm run typecheck(tsc --noEmit) which completed successfully.npm test(Vitest) and all tests passed including unit tests fornormalizeOidandOidPolicyServiceand integration tests for health and blocked-OID behavior.npm run build(tsc -p tsconfig.json) which completed successfully and produced the build artifacts.Codex Task