fix: use trust auth for docker-postgres - #1346
Conversation
POSTGRES_HOST_AUTH_METHOD=trust removes password auth for all connections. The password kept going out of sync after crash recovery, causing paperclip and other services to fail with 'password authentication failed'. Safe since postgres is only reachable from the host.
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughA single environment variable Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Mesa DescriptionTL;DRAdd What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request updates the PostgreSQL Docker startup script to enable the 'trust' authentication method, allowing passwordless access. A security review identified that this change, combined with the current port mapping, could expose the database to the network; it is recommended to restrict the port binding to '127.0.0.1' to ensure the database remains accessible only from the local host.
| -p "${HOST_PORT}:${CONTAINER_PORT}" \ | ||
| -e POSTGRES_DB=trails_api \ | ||
| -e POSTGRES_PASSWORD=postgres \ | ||
| -e POSTGRES_HOST_AUTH_METHOD=trust \ |
There was a problem hiding this comment.
Using trust authentication allows passwordless access to the database. While the PR description suggests this is safe, the port mapping -p "${HOST_PORT}:${CONTAINER_PORT}" on line 47 binds to 0.0.0.0 by default, exposing the database to the network. To ensure it is only reachable from localhost as intended, the port mapping should be restricted to 127.0.0.1. Additionally, consider updating the tests in spec/docker_postgres_spec.sh to reflect this change.
There was a problem hiding this comment.
Pull request overview
This PR updates the Home Manager–managed docker-postgres startup script to avoid password auth issues after container lifecycle events by configuring PostgreSQL to trust host connections.
Changes:
- Add
POSTGRES_HOST_AUTH_METHOD=trustto thedocker runenvironment for thepostgrescontainer.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -47,6 +47,7 @@ ensure_container() { | |||
| -p "${HOST_PORT}:${CONTAINER_PORT}" \ | |||
There was a problem hiding this comment.
POSTGRES_HOST_AUTH_METHOD=trust disables password checks for all TCP host connections. With the current -p "${HOST_PORT}:${CONTAINER_PORT}" publish syntax, Docker binds to 0.0.0.0 by default, so Postgres can become reachable from other machines on the network (depending on host firewall). To match the PR description (“only reachable from localhost/docker bridge”) and avoid unauthenticated remote access, bind the published port to loopback (e.g., 127.0.0.1) and/or avoid publishing the port entirely if only other containers need it.
| -p "${HOST_PORT}:${CONTAINER_PORT}" \ | |
| -p "127.0.0.1:${HOST_PORT}:${CONTAINER_PORT}" \ |
| -e POSTGRES_PASSWORD=postgres \ | ||
| -e POSTGRES_HOST_AUTH_METHOD=trust \ |
There was a problem hiding this comment.
This change introduces a new runtime behavior (trust auth) but the existing ShellSpec coverage for this script doesn’t assert it. Consider updating the docker-postgres spec to check for POSTGRES_HOST_AUTH_METHOD=trust (and ideally the chosen port bind strategy) so the security-sensitive configuration can’t regress silently.
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/services/docker-postgres/start-postgres.sh">
<violation number="1" location="home-manager/services/docker-postgres/start-postgres.sh:50">
P1: `POSTGRES_HOST_AUTH_METHOD=trust` removes authentication for host connections; with the published `5432` port this creates an unauthenticated database access path.</violation>
<violation number="2" location="home-manager/services/docker-postgres/start-postgres.sh:50">
P1: Setting `POSTGRES_HOST_AUTH_METHOD=trust` removes all authentication, but the port mapping on line 47 (`-p "${HOST_PORT}:${CONTAINER_PORT}"`) binds to `0.0.0.0` by default, exposing the database to the entire network without credentials. Restrict the bind address to localhost: `-p "127.0.0.1:${HOST_PORT}:${CONTAINER_PORT}"`</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| -p "${HOST_PORT}:${CONTAINER_PORT}" \ | ||
| -e POSTGRES_DB=trails_api \ | ||
| -e POSTGRES_PASSWORD=postgres \ | ||
| -e POSTGRES_HOST_AUTH_METHOD=trust \ |
There was a problem hiding this comment.
P1: POSTGRES_HOST_AUTH_METHOD=trust removes authentication for host connections; with the published 5432 port this creates an unauthenticated database access path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/docker-postgres/start-postgres.sh, line 50:
<comment>`POSTGRES_HOST_AUTH_METHOD=trust` removes authentication for host connections; with the published `5432` port this creates an unauthenticated database access path.</comment>
<file context>
@@ -47,6 +47,7 @@ ensure_container() {
-p "${HOST_PORT}:${CONTAINER_PORT}" \
-e POSTGRES_DB=trails_api \
-e POSTGRES_PASSWORD=postgres \
+ -e POSTGRES_HOST_AUTH_METHOD=trust \
"$IMAGE"
log "Container '$CONTAINER_NAME' created and started"
</file context>
| -e POSTGRES_HOST_AUTH_METHOD=trust \ | |
| -e POSTGRES_HOST_AUTH_METHOD=scram-sha-256 \ |
| -p "${HOST_PORT}:${CONTAINER_PORT}" \ | ||
| -e POSTGRES_DB=trails_api \ | ||
| -e POSTGRES_PASSWORD=postgres \ | ||
| -e POSTGRES_HOST_AUTH_METHOD=trust \ |
There was a problem hiding this comment.
P1: Setting POSTGRES_HOST_AUTH_METHOD=trust removes all authentication, but the port mapping on line 47 (-p "${HOST_PORT}:${CONTAINER_PORT}") binds to 0.0.0.0 by default, exposing the database to the entire network without credentials. Restrict the bind address to localhost: -p "127.0.0.1:${HOST_PORT}:${CONTAINER_PORT}"
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/docker-postgres/start-postgres.sh, line 50:
<comment>Setting `POSTGRES_HOST_AUTH_METHOD=trust` removes all authentication, but the port mapping on line 47 (`-p "${HOST_PORT}:${CONTAINER_PORT}"`) binds to `0.0.0.0` by default, exposing the database to the entire network without credentials. Restrict the bind address to localhost: `-p "127.0.0.1:${HOST_PORT}:${CONTAINER_PORT}"`</comment>
<file context>
@@ -47,6 +47,7 @@ ensure_container() {
-p "${HOST_PORT}:${CONTAINER_PORT}" \
-e POSTGRES_DB=trails_api \
-e POSTGRES_PASSWORD=postgres \
+ -e POSTGRES_HOST_AUTH_METHOD=trust \
"$IMAGE"
log "Container '$CONTAINER_NAME' created and started"
</file context>
* feat: add paperclip service (#1342) * feat: add paperclip service - Systemd service on kyber: runs `paperclipai run` via bun, depends on docker-postgres - Config via builtins.toJSON: external postgres on kyber, embedded on macOS - Setup script creates paperclip database on docker-postgres - Makefile target: `make systemctl-paperclip` * fix: add shellcheck disable and shell test coverage for paperclip * fix: correct shellcheck disable directive syntax * refactor: use config.template.json pattern for paperclip * refactor: rename setup.sh to hydrate.sh for paperclip * test: add auto-switch hook tests and update coverage spec * fix: remove EnvironmentFile from paperclip service (#1344) * fix: paperclip authenticated mode and required config fields (#1345) * fix: remove EnvironmentFile from paperclip service * fix: use authenticated mode on kyber, add required meta/logging fields * fix: add allowedHostnames for paperclip.shunkakinoki.com * fix: format config template json * fix: use trust auth for docker-postgres (#1346) POSTGRES_HOST_AUTH_METHOD=trust removes password auth for all connections. The password kept going out of sync after crash recovery, causing paperclip and other services to fail with 'password authentication failed'. Safe since postgres is only reachable from the host. * fix: use k8s postgres via DATABASE_URL, remove docker-postgres dep, add authenticated mode (#1347) * fix: use bun runtime for paperclip (pino-http node crash), k8s postgres via DATABASE_URL (#1348) * fix: use nix-profile bun path for paperclip service (#1350) * fix: add caret prefix to paperclipai dependency (#1349) * fix: use extra-substituters to avoid untrusted user warnings (#1351) * fix: run paperclip from cloned repo via pnpm dev:once (#1352) * fix: run paperclip from cloned repo via pnpm dev:once The global bun install flattens pino@10 + pino-http@10.5 together, but pino-http needs pino@9. The repo lockfile resolves this correctly with nested dependencies. Running from the repo avoids the crash. * fix: use bun run server/src/index.ts instead of pnpm dev:once * fix: pin pino@9.14.0 override, run paperclipai from dotfiles node_modules The bun flat hoisting was resolving pino@10 which is incompatible with pino-http@10.5. Pinning pino to 9.14.0 via overrides matches the paperclip repo's lockfile resolution and fixes the crash. * fix: use global bun paperclipai with pino override (#1353) * fix: use global ~/.bun/bin/paperclipai with pino override Propagate overrides from dotfiles package.json to ~/.bun/install/global/ so the global binary resolves pino@9.14.0 correctly. * test: add tests for npm-globals dependency overrides * fix: resolve GitHub Actions failures and code review issues - Fix non-portable \s regex to [[:space:]] in auto-switch.sh (shfmt compat) - Add jq dependency check alongside cswap - Use printf instead of echo for safer output - Fix claude-swap version from >=1.1.5 (non-existent) to >=0.7.1 - Add auto-switch.sh to Nix deployment config (default.nix) - Sort covered_scripts list alphabetically in coverage_spec.sh https://claude.ai/code/session_012GyQBesQGF1asTfKebWyLM --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
Add
POSTGRES_HOST_AUTH_METHOD=trustto the docker-postgres container.Problem
POSTGRES_PASSWORDonly sets the password on first container init. After crash recovery, the password in the auth system gets out of sync, causingpassword authentication failederrors that crash-loop paperclip and any other service using the postgres connection.Fix
Trust all connections — no password needed. Safe since this postgres is only reachable from localhost/docker bridge, not exposed externally.
Note
Existing container needs to be recreated for this to take effect:
docker stop postgres && docker rm postgres make systemctl-docker-postgresSummary by cubic
Switch
docker-postgresto trust auth by settingPOSTGRES_HOST_AUTH_METHOD=trust. This prevents password mismatches after crash recovery that caused "password authentication failed" and crash loops in dependent services.docker stop postgres && docker rm postgres, thenmake systemctl-docker-postgres.Written for commit 68d1293. Summary will update on new commits.