Update - #197
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the production Docker Compose configuration with debug/development defaults and moves Nginx access from localhost:8080 to host port 8081, increasing accessibility while changing the stack’s default runtime behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for your first pull request to Wardrowbe! Please make sure you've read the CONTRIBUTING guide, especially the PR title format section. A maintainer will review this soon. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docker-compose.prod.yml" line_range="50" />
<code_context>
- "host.docker.internal:host-gateway"
- "${OIDC_HOST:-localhost}:${OIDC_HOST_IP:-127.0.0.1}"
environment:
+ DEBUG: ${DEBUG:-true}
DATABASE_URL: postgresql+asyncpg://${POSTGRES_USER:-wardrobe}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB:-wardrobe}
REDIS_URL: redis://redis:6379
</code_context>
<issue_to_address>
**🚨 issue (security):** The production backend starts with DEBUG enabled whenever `DEBUG` is absent, exposing FastAPI's `/docs`, `/redoc`, and `/openapi.json` endpoints in the production deployment.
**Triggers:** When the deployment does not explicitly set DEBUG=false.
**Suggested fix:** Default DEBUG to false in the production compose file and require operators to opt into it explicitly.
</issue_to_address>
### Comment 2
<location path="docker-compose.prod.yml" line_range="150" />
<code_context>
NEXTAUTH_URL: ${NEXTAUTH_URL:-https://localhost:3000}
NEXTAUTH_SECRET: ${NEXTAUTH_SECRET:?Set NEXTAUTH_SECRET in .env}
NEXT_PUBLIC_API_URL: ""
+ DEV_MODE: ${DEV_MODE:-true}
# OIDC
OIDC_ISSUER_URL: ${OIDC_ISSUER_URL:-}
</code_context>
<issue_to_address>
**🚨 issue (security):** The production frontend always registers the unrestricted `dev-credentials` NextAuth provider by default. With OIDC configured and a non-default backend secret, users can select Dev Login, but `/api/v1/auth/sync` rejects the resulting login with 503; with the default secret, this creates an authentication bypass accepting any email and name.
**Triggers:** When DEV_MODE is not explicitly set to false.
**Suggested fix:** Default DEV_MODE to false in the production compose file and enable it only in local development.
</issue_to_address>
### Comment 3
<location path="docker-compose.prod.yml" line_range="172-174" />
<code_context>
container_name: wardrobe_nginx
restart: unless-stopped
ports:
- - "127.0.0.1:8080:80"
+ - "8081:80"
volumes:
- ./nginx/nginx.conf:/etc/nginx/nginx.conf:ro
</code_context>
<issue_to_address>
**issue (broader_impact):** The nginx listener changes from host-local port 8080 to host-wide port 8081. This breaks existing reverse proxies and bookmarks targeting `127.0.0.1:8080`, while the new all-interface binding allows clients to reach nginx directly and bypass the external authentication proxy described immediately above the service.
**Triggers:** When an external proxy is configured for the previous localhost:8080 endpoint, or when the host is reachable from an untrusted network.
**Suggested fix:** Preserve the expected port and bind it to localhost, or update every proxy configuration and explicitly ensure direct access cannot bypass authentication.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and the production stack now defaults DEBUG and DEV_MODE to true and binds nginx on all host interfaces at port 8081, potentially exposing development behavior or sensitive diagnostics to network clients. Reverting stops the exposure, but any access or information disclosed while it is live cannot be recovered, and these defaults take effect for every deployment that does not override them.
Blocking findings: docker-compose.prod.yml:50, docker-compose.prod.yml:150, docker-compose.prod.yml:174
| - "host.docker.internal:host-gateway" | ||
| - "${OIDC_HOST:-localhost}:${OIDC_HOST_IP:-127.0.0.1}" | ||
| environment: | ||
| DEBUG: ${DEBUG:-true} |
There was a problem hiding this comment.
🚨 issue (security): The production backend starts with DEBUG enabled whenever DEBUG is absent, exposing FastAPI's /docs, /redoc, and /openapi.json endpoints in the production deployment.
Triggers: When the deployment does not explicitly set DEBUG=false.
Suggested fix: Default DEBUG to false in the production compose file and require operators to opt into it explicitly.
| NEXTAUTH_URL: ${NEXTAUTH_URL:-https://localhost:3000} | ||
| NEXTAUTH_SECRET: ${NEXTAUTH_SECRET:?Set NEXTAUTH_SECRET in .env} | ||
| NEXT_PUBLIC_API_URL: "" | ||
| DEV_MODE: ${DEV_MODE:-true} |
There was a problem hiding this comment.
🚨 issue (security): The production frontend always registers the unrestricted dev-credentials NextAuth provider by default. With OIDC configured and a non-default backend secret, users can select Dev Login, but /api/v1/auth/sync rejects the resulting login with 503; with the default secret, this creates an authentication bypass accepting any email and name.
Triggers: When DEV_MODE is not explicitly set to false.
Suggested fix: Default DEV_MODE to false in the production compose file and enable it only in local development.
| - "127.0.0.1:8080:80" | ||
| - "8081:80" | ||
| volumes: | ||
| - ./nginx/nginx.conf:/etc/nginx/nginx.conf:ro |
There was a problem hiding this comment.
issue (broader_impact): The nginx listener changes from host-local port 8080 to host-wide port 8081. This breaks existing reverse proxies and bookmarks targeting 127.0.0.1:8080, while the new all-interface binding allows clients to reach nginx directly and bypass the external authentication proxy described immediately above the service.
Triggers: When an external proxy is configured for the previous localhost:8080 endpoint, or when the host is reachable from an untrusted network.
Suggested fix: Preserve the expected port and bind it to localhost, or update every proxy configuration and explicitly ensure direct access cannot bypass authentication.
2 participants
Description
Related Issue
Type of Change
Checklist
Testing
Test Environment
Tests Performed
Screenshots (if applicable)
Additional Notes
Summary by Sourcery
Adjust production Compose defaults and service exposure for easier local development and debugging.
Enhancements:
Deployment: