fix(antseed): parse a libpq kv-conninfo DATABASE_URL in the node writers#39
fix(antseed): parse a libpq kv-conninfo DATABASE_URL in the node writers#39jmlago wants to merge 1 commit into
Conversation
The prod secret hands DATABASE_URL as a libpq KEYWORD/VALUE conninfo (host=… port=… dbname=… user=… password='…' sslmode=require) — the form psycopg uses on the router/ingress. node-postgres's `connectionString` only parses a postgres:// URL, so the sidecar resolved host "base" and died with ENOTFOUND, leaving peer_offers / buyer_status empty after the host-store flip (#38). Add store.pgConfig(): pass a URL through (dev/compose), otherwise parse the kv conninfo into a pg config object (sslmode -> ssl). write-market.js, write-status.js and control.js use it. Verified: write-market.js writes peer_offers against a kv-conninfo DATABASE_URL pointed at the compose Postgres.
📝 WalkthroughWalkthrough
ChangespgConfig helper and adoption
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.0)antseed/control.jsFile contains syntax errors that prevent linting: Line 30: Illegal return statement outside of a function 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@antseed/store.js`:
- Line 27: The DSN check currently uses a generic substring test, which can
misclassify libpq conninfo strings when a quoted value contains “://”. Update
the DSN normalization logic in the store helper to detect URLs by anchoring on
the actual scheme at the start of the string, and only return {
connectionString: dsn } for real URI-style DSNs while leaving key/value conninfo
untouched.
- Line 40: The pgConfig() sslmode handling in store.js is too permissive because
every value other than "disable" is downgraded to { rejectUnauthorized: false },
which breaks stricter modes. Update the sslmode branch in pgConfig() to either
map each supported mode explicitly (including verify-ca and verify-full with
proper certificate verification behavior) or throw/reject unsupported values
instead of silently disabling verification.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5bdb976-1253-4e73-8e32-6cae3a43aa84
📒 Files selected for processing (4)
antseed/control.jsantseed/store.jsantseed/write-market.jsantseed/write-status.js
| // dies with ENOTFOUND, leaving peer_offers/buyer_status empty. | ||
| function pgConfig() { | ||
| const dsn = process.env.DATABASE_URL || ""; | ||
| if (!dsn || dsn.includes("://")) return { connectionString: dsn }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Anchor URL detection to the DSN scheme.
dsn.includes("://") misclassifies valid libpq conninfo if a quoted value contains ://, such as a generated password. That would send the whole key/value string back through connectionString and reintroduce the connection failure this helper is meant to avoid.
Proposed fix
- if (!dsn || dsn.includes("://")) return { connectionString: dsn };
+ if (!dsn || /^[a-z][a-z0-9+.-]*:\/\//i.test(dsn.trimStart())) {
+ return { connectionString: dsn };
+ }📝 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.
| if (!dsn || dsn.includes("://")) return { connectionString: dsn }; | |
| if (!dsn || /^[a-z][a-z0-9+.-]*:\/\//i.test(dsn.trimStart())) { | |
| return { connectionString: dsn }; | |
| } |
🤖 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 `@antseed/store.js` at line 27, The DSN check currently uses a generic
substring test, which can misclassify libpq conninfo strings when a quoted value
contains “://”. Update the DSN normalization logic in the store helper to detect
URLs by anchoring on the actual scheme at the start of the string, and only
return { connectionString: dsn } for real URI-style DSNs while leaving key/value
conninfo untouched.
| else if (key === "dbname") cfg.database = val; | ||
| else if (key === "user") cfg.user = val; | ||
| else if (key === "password") cfg.password = val; | ||
| else if (key === "sslmode") cfg.ssl = val === "disable" ? false : { rejectUnauthorized: false }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether repository-managed DATABASE_URL examples or manifests use stricter sslmode values.
rg -n "sslmode=(verify-ca|verify-full|require|disable|prefer|allow)" .Repository: genlayerlabs/unhardcoded
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== antseed/store.js (relevant section) ==\n'
cat -n antseed/store.js | sed -n '1,140p'
printf '\n== Search for sslmode handling and postgres URL usage ==\n'
rg -n "sslmode|DATABASE_URL|postgres://|postgresql://" antseed -g '!**/node_modules/**' || true
printf '\n== Search for any code paths that distinguish verify-ca / verify-full ==\n'
rg -n "verify-ca|verify-full|rejectUnauthorized|ssl\s*=" antseed -g '!**/node_modules/**' || trueRepository: genlayerlabs/unhardcoded
Length of output: 3717
Fail closed for stricter sslmode values. pgConfig() maps every non-disable value to { rejectUnauthorized: false }, so verify-ca and verify-full would silently lose certificate verification. Either preserve those modes or reject unsupported values instead of downgrading them.
🤖 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 `@antseed/store.js` at line 40, The pgConfig() sslmode handling in store.js is
too permissive because every value other than "disable" is downgraded to {
rejectUnauthorized: false }, which breaks stricter modes. Update the sslmode
branch in pgConfig() to either map each supported mode explicitly (including
verify-ca and verify-full with proper certificate verification behavior) or
throw/reject unsupported values instead of silently disabling verification.
|
superseded by #40 |
1 participant
The bug (live in prod after #38)
After the host-store flip, the antseed sidecar can't reach Postgres:
peer_offers/buyer_statusstay empty → antseed marketplace degraded (fail-soft to other providers).Root cause: the prod
DATABASE_URLsecret is a libpq keyword/value conninfo (host=… port=… dbname=… user=… password='…' sslmode=require) — chosen in the infra wiring (#165) to dodge password percent-encoding, and the form psycopg uses on the router/ingress. But the sidecar is node-postgres, whoseconnectionStringonly parses apostgres://URL — it mis-reads the conninfo and resolves hostbase.The fix
store.pgConfig(): pass a URL through untouched (dev/compose), otherwise parse the kv conninfo into a node-postgres config object (dbname→database,sslmode→ssl).write-market.js/write-status.js/control.jsuse it.Verified:
pgConfig()parses the prod-shaped conninfo correctly and passes URLs through;write-market.jswith a kv-conninfoDATABASE_URLwritespeer_offersagainst the compose Postgres. Node-only change (router/ingress psycopg path unaffected).Follow-up to #38.
Summary by CodeRabbit