Feature/go db pool 893#1018
Conversation
* feat(backend): add Go CI workflow and Docker setup * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(ci): address backend workflow and Docker review comments * fix(review): address CodeRabbit suggestions * fix(ci): update workflow branch target --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* feat: add PostgreSQL repository abstraction layer * refactor: improve repository abstraction contract
…199#509) * feat: initialize Go backend module and JWT auth middleware * fix: address CodeRabbit review feedback on JWT version, timeouts, and missing secrets
* Setup Go backend module structure for migration * Fix Go backend structure * Restore Go module to backend root
test: add Go backend auth and middleware coverage
👷 Deploy request for docmagic-muneer pending review.Visit the deploys page to approve it
|
👷 Deploy request for docmagic1 pending review.Visit the deploys page to approve it
|
|
@NayansiDupare is attempting to deploy a commit to the muneerali199's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 30 minutes and 27 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces a complete Go HTTP backend with Chi router, JWT/Supabase auth middleware, pgxpool database connection layer with retry backoff, shared error utilities, Docker multi-stage build, and a GitHub Actions CI workflow. It also adds four Next.js observability routes, a TypeScript Supabase credits repository, and migration strategy documentation. ChangesGo Backend Foundation
Next.js Observability Routes and TypeScript Credits Repository
Sequence Diagram(s)sequenceDiagram
participant Client
participant RequireAuth as RequireAuth Middleware
participant JWTLib as golang-jwt/jwt
participant AuthContext as auth.WithUser
participant APIHandler as /api/user Handler
participant DBPool as pgxpool
Client->>RequireAuth: GET /api/user (Authorization: Bearer <token>)
RequireAuth->>RequireAuth: os.Getenv(SUPABASE_JWT_SECRET)
RequireAuth->>JWTLib: ParseWithClaims(token, HS256)
JWTLib-->>RequireAuth: MapClaims or error
RequireAuth->>AuthContext: WithUser(ctx, &User{ID, Email, Role})
AuthContext-->>RequireAuth: enriched context
RequireAuth->>APIHandler: ServeHTTP(w, r with user context)
APIHandler->>DBPool: (future queries)
APIHandler-->>Client: 200 JSON {user}
sequenceDiagram
participant main
participant LoadConfig as database.LoadConfig
participant NewPool as database.NewPool
participant pgxpool
participant httpServer as http.Server
participant OS
main->>LoadConfig: read DATABASE_URL + DB_* env vars
LoadConfig-->>main: *Config
main->>NewPool: NewPool(ctx, cfg)
loop retry with backoff
NewPool->>pgxpool: pool.Ping(ctx)
pgxpool-->>NewPool: ok / error
end
NewPool-->>main: *pgxpool.Pool
main->>httpServer: ListenAndServe (goroutine)
OS-->>main: SIGINT/SIGTERM
main->>httpServer: Shutdown(30s ctx)
main->>pgxpool: pool.Close()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/api/generate/resume/route.ts (1)
292-299:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix schema mismatch in credit_usage_log insert.
Line 296 uses the column name
action, but thecredit_usage_logtable schema expectsaction_type. This mismatch will cause the insert to silently fail, breaking credit usage audit logs and analytics.🔧 Proposed fix
await supabaseAdmin .from('credit_usage_log') .insert({ user_id: user.id, - action: 'resume', + action_type: 'resume', credits_used: creditCost, metadata: { prompt_length: sanitizedPrompt.length } });🤖 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 `@app/api/generate/resume/route.ts` around lines 292 - 299, The credit_usage_log insert statement in the resume route uses the incorrect column name `action` when it should use `action_type` to match the table schema. Update the insert object in the supabaseAdmin.from('credit_usage_log').insert() call to change the key from `action` to `action_type` while keeping the value as `'resume'` to ensure credit usage is properly logged and the insert succeeds.Source: Learnings
.env.example (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
GEMINI_API_KEYis duplicated in the example env file.Having the same key twice is error-prone when users edit one occurrence and miss the other.
Also applies to: 41-41
🤖 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 @.env.example at line 1, The GEMINI_API_KEY configuration variable is duplicated in the .env.example file, appearing at both line 1 and line 41. Remove one of the duplicate GEMINI_API_KEY entries to ensure this configuration variable appears only once in the example file. This prevents confusion and potential inconsistencies when users are setting up their environment variables by copying from the example file.
🧹 Nitpick comments (4)
backend/Makefile (1)
1-8: ⚡ Quick winDeclare Make targets as
.PHONYto avoid accidental no-op runs.If files named
run,test, orlintappear, Make may skip these targets unexpectedly.Proposed patch
+.PHONY: run test lint + run: go run go/cmd/server/main.go🤖 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 `@backend/Makefile` around lines 1 - 8, Add a .PHONY declaration at the top of the Makefile to declare the run, test, and lint targets as phony targets. This prevents Make from treating them as file-based targets and ensures they execute unconditionally even if files with these names exist in the directory. Place the .PHONY directive before the target definitions listing all three target names.Source: Linters/SAST tools
backend/pkg/auth/middleware_test.go (1)
49-121: ⚡ Quick winAdd a test for the missing
SUPABASE_JWT_SECRETbranch.
RequireAuthhas a distinct Line 30-37 path that returns HTTP 500 when the secret is unset, but this branch is currently untested. Adding one case will lock in startup/config error behavior and prevent silent regressions.🤖 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 `@backend/pkg/auth/middleware_test.go` around lines 49 - 121, Add a new test function to cover the missing branch where SUPABASE_JWT_SECRET is not set. Create a test (e.g., TestRequireAuthReturnErrorWhenSecretNotSet) that does NOT set the SUPABASE_JWT_SECRET environment variable, then call runRequireAuthRequest with any token and verify that the response status code is http.StatusInternalServerError (500). This will ensure the configuration error path in RequireAuth is properly tested and prevent regressions in startup/config error handling.backend/go/middleware/middleware.go (1)
18-20: Use an unexported typed key instead of raw string for context values.The
"request_id"key in line 18 is collision-prone. Replace with an unexported typed key and provide a getter helper function:Suggested refactor
+type requestIDContextKey struct{} + +var requestIDKey requestIDContextKey + +func RequestIDFromContext(ctx context.Context) (string, bool) { + v, ok := ctx.Value(requestIDKey).(string) + return v, ok +} + func RequestID(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requestID := uuid.NewString() @@ - ctx := context.WithValue(r.Context(), "request_id", requestID) + ctx := context.WithValue(r.Context(), requestIDKey, requestID)🤖 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 `@backend/go/middleware/middleware.go` around lines 18 - 20, Replace the raw string `"request_id"` used as a context key in the context.WithValue call with an unexported typed key to avoid collision risks. Create an unexported type (using an empty struct or a custom type) to serve as the context key, update the context.WithValue call to use this typed key instead of the string, and provide a getter helper function that accepts a context and returns the request_id value using this typed key for retrieving the value safely throughout the codebase.backend/go/internal/database/pool_test.go (1)
10-12: ⚡ Quick winHarden env-based tests with
t.Setenvand assert idle-time default.Use
t.Setenvto avoid cross-test env leakage, and add an assertion forMaxConnIdleTimein the defaults test so all documented defaults are covered.Suggested fix
func TestLoadConfig_Defaults(t *testing.T) { - os.Setenv("DATABASE_URL", "postgres://user:pass@localhost:5432/dbname") - defer os.Unsetenv("DATABASE_URL") + t.Setenv("DATABASE_URL", "postgres://user:pass@localhost:5432/dbname") @@ if cfg.MaxConnLifetime != 30*time.Minute { t.Errorf("expected MaxConnLifetime to be 30m, got %v", cfg.MaxConnLifetime) } + if cfg.MaxConnIdleTime != 15*time.Minute { + t.Errorf("expected MaxConnIdleTime to be 15m, got %v", cfg.MaxConnIdleTime) + } } func TestLoadConfig_Overrides(t *testing.T) { - os.Setenv("DATABASE_URL", "postgres://user:pass@localhost:5432/dbname") - os.Setenv("DB_MAX_CONNS", "25") - os.Setenv("DB_MIN_CONNS", "5") - os.Setenv("DB_MAX_CONN_LIFETIME", "1h") - os.Setenv("DB_MAX_CONN_IDLE_TIME", "10m") - - defer func() { - os.Unsetenv("DATABASE_URL") - os.Unsetenv("DB_MAX_CONNS") - os.Unsetenv("DB_MIN_CONNS") - os.Unsetenv("DB_MAX_CONN_LIFETIME") - os.Unsetenv("DB_MAX_CONN_IDLE_TIME") - }() + t.Setenv("DATABASE_URL", "postgres://user:pass@localhost:5432/dbname") + t.Setenv("DB_MAX_CONNS", "25") + t.Setenv("DB_MIN_CONNS", "5") + t.Setenv("DB_MAX_CONN_LIFETIME", "1h") + t.Setenv("DB_MAX_CONN_IDLE_TIME", "10m")Also applies to: 24-27, 30-42
🤖 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 `@backend/go/internal/database/pool_test.go` around lines 10 - 12, Replace all instances of os.Setenv combined with defer os.Unsetenv throughout the pool_test.go file (including the section at lines 10-12, 24-27, and 30-42) with t.Setenv calls, which automatically handle cleanup and prevent environment variable leakage between tests. Additionally, locate the defaults test and add an assertion to verify that the MaxConnIdleTime field is set to its documented default value, ensuring comprehensive coverage of all default configuration values.
🤖 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 @.github/workflows/backend-ci.yml:
- Around line 4-7: The pull_request trigger in the workflow has an overly
restrictive branches filter that only allows the feature/go-backend branch,
preventing CI from running on pull requests from other branches. Remove or
modify the branches filter under the pull_request section to allow PRs from all
branches (or the relevant branches your team uses), unless there's a specific
reason to limit it to only feature/go-backend.
- Around line 26-32: The actions/checkout and actions/setup-go action references
use mutable version tags (v4 and v5 respectively) instead of pinned commit SHAs,
creating supply chain vulnerabilities. Replace both action references with their
full commit SHA hashes to ensure immutability and prevent potential malicious
code injection. Additionally, add a with parameter to the actions/checkout step
to disable credential persistence, preventing the GITHUB_TOKEN from being stored
in .git/config since this workflow only performs read-only git operations.
In `@backend/Dockerfile`:
- Around line 10-22: The Docker build path in the go build command is incorrect,
referencing ./cmd when the actual server entrypoint is under ./go/cmd/server.
Additionally, the runtime stage sets WORKDIR to /root/ but then switches to
appuser, which will not have permissions to access the root directory. Fix this
by updating the go build command to use the correct path to ./go/cmd/server, and
change the WORKDIR in the runtime stage to a directory that appuser has access
to (such as /app). Ensure the COPY command copies the built binary to match the
new WORKDIR path so that appuser can execute it properly.
In `@backend/go.mod`:
- Line 9: The github.com/jackc/pgx/v5 dependency in the go.mod file is pinned to
version v5.6.0, which contains critical memory-safety and SQL injection
vulnerabilities. Update the version number for github.com/jackc/pgx/v5 from
v5.6.0 to v5.9.2 or later in the go.mod file to address all known security
advisories, then run go mod tidy to update the go.sum file accordingly.
In `@backend/go/cmd/server/main.go`:
- Around line 119-125: The goroutine running srv.ListenAndServe() is calling
os.Exit(1) directly when an error occurs, which bypasses the normal shutdown
orchestration and prevents cleanup. Create an error channel to capture server
errors from the goroutine, send any non-nil errors to this channel instead of
calling os.Exit(1), and then handle the error in the main execution path
alongside signal handling so that all shutdown logic is centralized and cleanup
procedures execute properly.
- Around line 50-58: The database pool initialization uses an unbounded
context.Background() which can cause the application to hang indefinitely during
network or database connectivity issues. Replace the context.Background()
assignment with a context that has a timeout using context.WithTimeout, and pass
this bounded context to the database.NewPool function call. This ensures the
pool initialization will fail fast if the database is unreachable or
unresponsive.
In `@backend/go/internal/database/pool.go`:
- Around line 38-52: After parsing DB_MAX_CONNS and DB_MIN_CONNS from
environment variables and assigning the values to cfg.MaxConns and cfg.MinConns
respectively, add validation logic to ensure the pool size invariants are
satisfied. Validate that cfg.MaxConns is not negative, cfg.MinConns is not
negative, and crucially that cfg.MinConns does not exceed cfg.MaxConns. If any
invariant is violated, log a warning and use sensible default values to prevent
downstream startup or runtime failures.
- Around line 95-116: The database ping retry loop continues to wait for the
backoff delay even after the final retry attempt fails, adding unnecessary
startup delay. Add a check to determine if the current iteration is the last
retry (when i equals maxRetries - 1), and if the ping fails on that final
attempt, skip the backoff sleep by either breaking out of the loop or returning
the error directly instead of entering the select statement with
time.After(backoff).
In `@backend/go/README.md`:
- Around line 5-9: The bash code block in the README.md contains incorrect shell
syntax. Replace the `set` command with `export` for the SUPABASE_JWT_SECRET
variable so the environment variable is properly exported when running `make
run`. Additionally, add a second `export` statement for the required but
undocumented DATABASE_URL environment variable with an appropriate PostgreSQL
connection string (e.g., postgres://postgres:password@localhost:5432/postgres)
to complete the local setup instructions.
In `@backend/pkg/auth/middleware.go`:
- Around line 14-24: The ErrorResponse struct and writeError function in the
middleware are using an inconsistent error response schema that only includes an
error field. Update the ErrorResponse struct to match the shared error contract
defined in backend/go/utils/errors.go by including all required fields (error,
message, status). Then modify the writeError function to populate the
ErrorResponse struct with all the required fields according to the unified error
contract instead of only setting the error message.
In `@docker-compose.yml`:
- Around line 14-15: The docker-compose.yml service environment section is
missing the required DATABASE_URL environment variable that
database.LoadConfig() depends on. Add DATABASE_URL to the environment section
alongside the existing PORT configuration, setting it to the appropriate
database connection string for your application to start successfully.
In `@docs/migrations/postgres-repository-migration.md`:
- Around line 53-56: The Phase 3 bullet point "Introduce connection pooling and
transactional services" is misleading because connection pooling already exists
in the current Go backend foundation. Clarify this bullet point by rephrasing it
to indicate that Phase 3 is enhancing or optimizing the existing pooling for
PostgreSQL-native backend compatibility, rather than introducing pooling as a
new feature. This will eliminate ambiguity about whether pooling is being added
for the first time or is being adapted/upgraded as part of the migration.
In `@lib/repositories/credits-repository.ts`:
- Around line 1-6: The supabaseAdmin client creation lacks a server-only guard
despite using a sensitive service-role key, creating a risk of accidental
client-side exposure. Add the server-only package to your dependencies, then
import it at the very top of lib/repositories/credits-repository.ts before the
createClient call for supabaseAdmin. This will enforce that the module can only
be imported on the server, preventing future regressions if this module is
accidentally referenced in client-side code.
- Around line 37-51: The createUserCredits function throws an error when it
encounters a unique constraint violation (error code 23505) from concurrent
requests, making it non-idempotent. Modify the error handling logic to check if
the error code is 23505 (unique constraint violation on the user_id field), and
instead of throwing, call the existing getUserCredits(userId) method to fetch
and return the existing record. This allows the function to safely handle
duplicate or concurrent creation attempts without failing on subsequent
requests.
---
Outside diff comments:
In @.env.example:
- Line 1: The GEMINI_API_KEY configuration variable is duplicated in the
.env.example file, appearing at both line 1 and line 41. Remove one of the
duplicate GEMINI_API_KEY entries to ensure this configuration variable appears
only once in the example file. This prevents confusion and potential
inconsistencies when users are setting up their environment variables by copying
from the example file.
In `@app/api/generate/resume/route.ts`:
- Around line 292-299: The credit_usage_log insert statement in the resume route
uses the incorrect column name `action` when it should use `action_type` to
match the table schema. Update the insert object in the
supabaseAdmin.from('credit_usage_log').insert() call to change the key from
`action` to `action_type` while keeping the value as `'resume'` to ensure credit
usage is properly logged and the insert succeeds.
---
Nitpick comments:
In `@backend/go/internal/database/pool_test.go`:
- Around line 10-12: Replace all instances of os.Setenv combined with defer
os.Unsetenv throughout the pool_test.go file (including the section at lines
10-12, 24-27, and 30-42) with t.Setenv calls, which automatically handle cleanup
and prevent environment variable leakage between tests. Additionally, locate the
defaults test and add an assertion to verify that the MaxConnIdleTime field is
set to its documented default value, ensuring comprehensive coverage of all
default configuration values.
In `@backend/go/middleware/middleware.go`:
- Around line 18-20: Replace the raw string `"request_id"` used as a context key
in the context.WithValue call with an unexported typed key to avoid collision
risks. Create an unexported type (using an empty struct or a custom type) to
serve as the context key, update the context.WithValue call to use this typed
key instead of the string, and provide a getter helper function that accepts a
context and returns the request_id value using this typed key for retrieving the
value safely throughout the codebase.
In `@backend/Makefile`:
- Around line 1-8: Add a .PHONY declaration at the top of the Makefile to
declare the run, test, and lint targets as phony targets. This prevents Make
from treating them as file-based targets and ensures they execute
unconditionally even if files with these names exist in the directory. Place the
.PHONY directive before the target definitions listing all three target names.
In `@backend/pkg/auth/middleware_test.go`:
- Around line 49-121: Add a new test function to cover the missing branch where
SUPABASE_JWT_SECRET is not set. Create a test (e.g.,
TestRequireAuthReturnErrorWhenSecretNotSet) that does NOT set the
SUPABASE_JWT_SECRET environment variable, then call runRequireAuthRequest with
any token and verify that the response status code is
http.StatusInternalServerError (500). This will ensure the configuration error
path in RequireAuth is properly tested and prevent regressions in startup/config
error handling.
🪄 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: 4d9d7010-d26b-46bc-94f4-5e0d706ca3d5
⛔ Files ignored due to path filters (2)
backend/go.sumis excluded by!**/*.sumpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (31)
.env.example.github/workflows/backend-ci.ymlapp/api/generate/resume/route.tsapp/api/healthz/route.tsapp/api/readyz/route.tsapp/api/version/route.tsapp/healthz/route.tsapp/readyz/route.tsapp/version/route.tsbackend/.dockerignorebackend/Dockerfilebackend/Makefilebackend/go.modbackend/go/README.mdbackend/go/cmd/server/main.gobackend/go/internal/database/database.gobackend/go/internal/database/pool.gobackend/go/internal/database/pool_test.gobackend/go/middleware/middleware.gobackend/go/middleware/middleware_test.gobackend/go/router/router.gobackend/go/utils/errors.gobackend/go/utils/errors_test.gobackend/main.gobackend/pkg/auth/context.gobackend/pkg/auth/middleware.gobackend/pkg/auth/middleware_test.godocker-compose.ymldocs/go-migration/router-middleware-foundation.mddocs/migrations/postgres-repository-migration.mdlib/repositories/credits-repository.ts
| pull_request: | ||
| branches: | ||
| - feature/go-backend | ||
| paths: |
There was a problem hiding this comment.
CI won’t run for most pull requests due to branch filter.
Lines 5-6 restrict PR CI to feature/go-backend, so PRs from other branches can bypass this workflow.
Proposed patch
pull_request:
branches:
- - feature/go-backend
+ - main🤖 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 @.github/workflows/backend-ci.yml around lines 4 - 7, The pull_request
trigger in the workflow has an overly restrictive branches filter that only
allows the feature/go-backend branch, preventing CI from running on pull
requests from other branches. Remove or modify the branches filter under the
pull_request section to allow PRs from all branches (or the relevant branches
your team uses), unless there's a specific reason to limit it to only
feature/go-backend.
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: "1.22" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/backend-ci.yml | sed -n '20,40p'Repository: Muneerali199/Draftdeckai
Length of output: 647
🌐 Web query:
GitHub Actions pinning commit SHA security best practice NIST
💡 Result:
Pinning GitHub Actions to a full-length commit SHA is a widely recognized security best practice for mitigating supply chain risks [1][2][3]. This approach ensures that workflows execute immutable, verified code, preventing attacks where an adversary modifies an existing tag or branch to inject malicious payloads [2][4][5]. Industry Alignment and NIST Guidance While NIST publications, such as SP 800-204D, do not explicitly mandate "commit SHA pinning" as a single technical instruction, they provide the foundational framework for software supply chain security [6][7][8]. These documents emphasize critical security objectives that SHA pinning directly supports, including: 1. Software Supply Chain Security (SSDF): NIST guidance stresses the importance of integrity, provenance, and the mitigation of risks associated with external dependencies [9][10]. Pinning to a commit SHA provides cryptographic assurance of code integrity, aligning with the requirement to verify the provenance and immutability of build components [2][5]. 2. Secure CI/CD Pipelines: NIST SP 800-204D outlines strategies for integrating security into DevSecOps, specifically addressing the need to secure the "build" stage of pipelines [6][8]. By treating actions as immutable dependencies, organizations reduce the attack surface against unauthorized code changes, which is a key control in securing CI/CD infrastructure [5][7]. Practical Implementation Best Practices To maintain security while managing the overhead of pinned dependencies, organizations typically adopt the following practices: - Enforce at Policy Level: GitHub allows administrators to require full-length commit SHA pinning at the organization or repository level [1][11][5]. When enabled, any workflow attempting to use a mutable tag or branch will fail, enforcing compliance across the organization [11][5]. - Automate Updates: Because SHAs are static, they do not automatically receive security updates. Organizations are strongly encouraged to use automated dependency management tools—such as Dependabot or Renovate—to monitor and create pull requests for updated SHAs [4][5][3]. - Human-Readable Documentation: When pinning to a SHA, include the original version tag as a comment in the workflow file (e.g., uses: actions/checkout@sha123 # v4.0.0) [5]. This maintains context for developers and security reviewers regarding which version is intended [5]. - Recursive Pinning: If you maintain custom actions that consume other actions, you must also pin your internal sub-action references to SHAs. This ensures that your action remains compatible with organizations that enforce strict SHA-pinning policies [11]. By combining cryptographic pinning with automated dependency updates, organizations satisfy the rigorous security standards recommended by NIST for supply chain integrity within modern CI/CD pipelines [5][9][7].
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 3: https://github.blog/security/supply-chain-security/securing-the-open-source-supply-chain-across-github/
- 4: https://github.com/kubernetes/community/blob/main/github-management/github-actions-policy.md
- 5: https://architecture.lullabot.com/adr/20260505-pin-github-actions-to-hash/
- 6: https://csrc.nist.gov/pubs/sp/800/204/d/final
- 7: https://www.stingrai.io/blog/github-actions-security-checklist
- 8: https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204D.pdf
- 9: https://wellarchitected.github.com/library/scenarios/nist-ssdf-implementation/
- 10: https://www.nist.gov/itl/executive-order-14028-improving-nations-cybersecurity/software-supply-chain-security-guidance-16
- 11: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
🌐 Web query:
GitHub Actions persist-credentials security recommendation
💡 Result:
The persist-credentials input in the official actions/checkout GitHub Action controls whether the action persists the authentication token in the local git configuration of the runner [1][2]. Key security recommendation: It is a widely accepted security best practice to set persist-credentials: false unless your workflow explicitly requires the git CLI to perform authenticated operations like git push or git tag [3][4][5]. Why this is a security risk: - Default Behavior: The input defaults to true, meaning the authentication token (e.g., GITHUB_TOKEN) is automatically stored in the.git/config file of the checked-out repository [1][6]. - Implicit Access: Any subsequent steps in the same job can read this file, potentially exposing the token to malicious or compromised third-party actions [7][8][5]. - Accidental Exposure: If the workflow uploads the repository workspace as a build artifact (e.g., using actions/upload-artifact), the.git/config file—and the sensitive token inside it—may be inadvertently leaked to external environments [9][6]. How to implement: To disable this behavior, explicitly set the input in your workflow YAML: - uses: actions/checkout@v4 with: persist-credentials: false If you do not require git push or similar authenticated git operations, disabling this is recommended to adhere to the principle of least privilege and reduce the attack surface of your CI/CD pipeline [10][4][5].
Citations:
- 1: https://github.com/actions/checkout
- 2: actions/checkout@c170eef
- 3: https://release-plz.dev/docs/github/persist-credentials
- 4: https://cheatsheetseries.owasp.org/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.html
- 5: https://actsense.dev/vulnerabilities/unsafe_checkout/
- 6: https://yossarian.net/til/post/actions-checkout-can-leak-github-credentials/
- 7: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 8: https://github.com/suzuki-shunsuke/ghalint/blob/main/docs/policies/013.md
- 9: https://sisaku-security.github.io/lint/docs/rules/artipacked/
- 10: https://github.com/github/docs/blob/main/content/actions/reference/security/secure-use.md
Pin GitHub Actions to commit SHA and disable credential persistence.
Lines 27–30 use mutable tag refs (v4, v5) instead of pinned commit SHAs, creating a supply chain vulnerability where tag modifications could inject malicious code. Additionally, line 27 persists credentials by default, storing the GITHUB_TOKEN in .git/config where subsequent steps can access it—an unnecessary exposure given this workflow only performs read-only git operations.
Suggested hardening
- name: Checkout repository
- uses: actions/checkout@v4
+ uses: actions/checkout@a5ac7e51b41094c7d81cd2926d412afa3da67df2 # v4.1.6
+ with:
+ persist-credentials: false
- name: Set up Go
- uses: actions/setup-go@v5
+ uses: actions/setup-go@0a12ed9d6470824250f19a37f27f20f213e2f545 # v5.0.0📝 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.
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| - name: Set up Go | |
| uses: actions/setup-go@v5 | |
| with: | |
| go-version: "1.22" | |
| - name: Checkout repository | |
| uses: actions/checkout@a5ac7e51b41094c7d81cd2926d412afa3da67df2 # v4.1.6 | |
| with: | |
| persist-credentials: false | |
| - name: Set up Go | |
| uses: actions/setup-go@0a12ed9d6470824250f19a37f27f20f213e2f545 # v5.0.0 | |
| with: | |
| go-version: "1.22" |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 26-27: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 27-27: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 30-30: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/backend-ci.yml around lines 26 - 32, The actions/checkout
and actions/setup-go action references use mutable version tags (v4 and v5
respectively) instead of pinned commit SHAs, creating supply chain
vulnerabilities. Replace both action references with their full commit SHA
hashes to ensure immutability and prevent potential malicious code injection.
Additionally, add a with parameter to the actions/checkout step to disable
credential persistence, preventing the GITHUB_TOKEN from being stored in
.git/config since this workflow only performs read-only git operations.
Source: Linters/SAST tools
| RUN CGO_ENABLED=0 GOOS=linux go build -o server ./cmd | ||
|
|
||
| # Runtime stage | ||
| FROM alpine:latest | ||
|
|
||
| WORKDIR /root/ | ||
|
|
||
| RUN adduser -D appuser | ||
|
|
||
| COPY --from=builder /app/server . | ||
|
|
||
| USER appuser | ||
|
|
There was a problem hiding this comment.
Container build/start path is broken (./cmd) and runtime directory is incompatible with appuser.
Line 10 builds ./cmd, but this backend’s server entrypoint is under go/cmd/server. Also, Lines 15-22 run as appuser with WORKDIR /root/, which can fail due to directory permissions.
Proposed patch
-RUN CGO_ENABLED=0 GOOS=linux go build -o server ./cmd
+RUN CGO_ENABLED=0 GOOS=linux go build -o /server ./go/cmd/server
# Runtime stage
FROM alpine:latest
-WORKDIR /root/
+WORKDIR /app
RUN adduser -D appuser
-COPY --from=builder /app/server .
+COPY --from=builder /server /app/server
+RUN chown -R appuser:appuser /app
USER appuser🤖 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 `@backend/Dockerfile` around lines 10 - 22, The Docker build path in the go
build command is incorrect, referencing ./cmd when the actual server entrypoint
is under ./go/cmd/server. Additionally, the runtime stage sets WORKDIR to /root/
but then switches to appuser, which will not have permissions to access the root
directory. Fix this by updating the go build command to use the correct path to
./go/cmd/server, and change the WORKDIR in the runtime stage to a directory that
appuser has access to (such as /app). Ensure the COPY command copies the built
binary to match the new WORKDIR path so that appuser can execute it properly.
| github.com/go-chi/chi/v5 v5.0.12 | ||
| github.com/golang-jwt/jwt/v5 v5.2.2 | ||
| github.com/google/uuid v1.6.0 | ||
| github.com/jackc/pgx/v5 v5.6.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the first patched github.com/jackc/pgx/v5 version for GO-2026-4771, GO-2026-4772, GHSA-9jj7-4m8r-rfcm, and GHSA-j88v-2chj-qfwx?
💡 Result:
The first patched version for the specified vulnerabilities in github.com/jackc/pgx/v5 is v5.9.0 for GO-2026-4771, GO-2026-4772, and GHSA-9jj7-4m8r-rfcm [1][2], and v5.9.2 for GHSA-j88v-2chj-qfwx [3]. Details: - GO-2026-4771, GO-2026-4772, and GHSA-9jj7-4m8r-rfcm (associated with memory-safety vulnerabilities in the pgproto3 component): These vulnerabilities were addressed in version v5.9.0 [1][4][2]. - GHSA-j88v-2chj-qfwx (associated with a SQL injection vulnerability): This vulnerability was addressed in version v5.9.2 [3].
Citations:
- 1: https://pkg.go.dev/vuln/GO-2026-4772
- 2: https://osv.dev/vulnerability/GHSA-9jj7-4m8r-rfcm
- 3: https://nvd.nist.gov/vuln/detail/CVE-2026-41889
- 4: https://pkg.go.dev/vuln/GO-2026-4771
Upgrade pgx/v5 to v5.9.2 or later before merge.
github.com/jackc/pgx/v5 v5.6.0 contains critical vulnerabilities: memory-safety issues (GO-2026-4771, GO-2026-4772, GHSA-9jj7-4m8r-rfcm) and SQL injection (GHSA-j88v-2chj-qfwx). Upgrade to v5.9.2, which addresses all known advisories.
🤖 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 `@backend/go.mod` at line 9, The github.com/jackc/pgx/v5 dependency in the
go.mod file is pinned to version v5.6.0, which contains critical memory-safety
and SQL injection vulnerabilities. Update the version number for
github.com/jackc/pgx/v5 from v5.6.0 to v5.9.2 or later in the go.mod file to
address all known security advisories, then run go mod tidy to update the go.sum
file accordingly.
Source: Linters/SAST tools
| ctx := context.Background() | ||
| dbCfg, err := database.LoadConfig() | ||
| if err != nil { | ||
| log.Fatalf("Failed to load database config: %v", err) | ||
| } | ||
|
|
||
| slog.Info("initializing database pool...") | ||
| pool, err := database.NewPool(ctx, dbCfg) | ||
| if err != nil { |
There was a problem hiding this comment.
Bound DB bootstrap with a timeout context.
Using context.Background() for pool initialization can leave startup hanging on network/DB stalls. Pass a bounded context into database.NewPool.
Suggested fix
- ctx := context.Background()
+ startupCtx, startupCancel := context.WithTimeout(context.Background(), 45*time.Second)
+ defer startupCancel()
dbCfg, err := database.LoadConfig()
@@
- pool, err := database.NewPool(ctx, dbCfg)
+ pool, err := database.NewPool(startupCtx, dbCfg)📝 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.
| ctx := context.Background() | |
| dbCfg, err := database.LoadConfig() | |
| if err != nil { | |
| log.Fatalf("Failed to load database config: %v", err) | |
| } | |
| slog.Info("initializing database pool...") | |
| pool, err := database.NewPool(ctx, dbCfg) | |
| if err != nil { | |
| startupCtx, startupCancel := context.WithTimeout(context.Background(), 45*time.Second) | |
| defer startupCancel() | |
| dbCfg, err := database.LoadConfig() | |
| if err != nil { | |
| log.Fatalf("Failed to load database config: %v", err) | |
| } | |
| slog.Info("initializing database pool...") | |
| pool, err := database.NewPool(startupCtx, dbCfg) | |
| if err != nil { |
🤖 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 `@backend/go/cmd/server/main.go` around lines 50 - 58, The database pool
initialization uses an unbounded context.Background() which can cause the
application to hang indefinitely during network or database connectivity issues.
Replace the context.Background() assignment with a context that has a timeout
using context.WithTimeout, and pass this bounded context to the database.NewPool
function call. This ensures the pool initialization will fail fast if the
database is unreachable or unresponsive.
| // ErrorResponse represents a standard JSON error returned by the API. | ||
| type ErrorResponse struct { | ||
| Error string `json:"error"` | ||
| } | ||
|
|
||
| // writeError Helper to write JSON error responses. | ||
| func writeError(w http.ResponseWriter, status int, message string) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(status) | ||
| json.NewEncoder(w).Encode(ErrorResponse{Error: message}) | ||
| } |
There was a problem hiding this comment.
Unify auth error responses with the shared API error contract.
Lines 14-24 emit {"error": ...} only, while backend/go/utils/errors.go defines a structured error payload (error, message, status). This creates inconsistent response schemas across endpoints/middleware and can break client-side error parsing during migration.
🤖 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 `@backend/pkg/auth/middleware.go` around lines 14 - 24, The ErrorResponse
struct and writeError function in the middleware are using an inconsistent error
response schema that only includes an error field. Update the ErrorResponse
struct to match the shared error contract defined in backend/go/utils/errors.go
by including all required fields (error, message, status). Then modify the
writeError function to populate the ErrorResponse struct with all the required
fields according to the unified error contract instead of only setting the error
message.
| environment: | ||
| PORT: 8080 No newline at end of file |
There was a problem hiding this comment.
DATABASE_URL is missing from service env, so startup will fail.
database.LoadConfig() requires DATABASE_URL; only PORT is provided here.
Proposed patch
environment:
PORT: 8080
+ DATABASE_URL: ${DATABASE_URL}📝 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.
| environment: | |
| PORT: 8080 | |
| environment: | |
| PORT: 8080 | |
| DATABASE_URL: ${DATABASE_URL} |
🤖 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 `@docker-compose.yml` around lines 14 - 15, The docker-compose.yml service
environment section is missing the required DATABASE_URL environment variable
that database.LoadConfig() depends on. Add DATABASE_URL to the environment
section alongside the existing PORT configuration, setting it to the appropriate
database connection string for your application to start successfully.
| - Prepare PostgreSQL-native backend compatibility | ||
| - Align repository contracts with future Go services | ||
| - Introduce connection pooling and transactional services | ||
|
|
There was a problem hiding this comment.
Clarify Phase 3 wording to avoid contradicting current stack state.
Line 55 says to “introduce connection pooling,” but pooling is already part of the Go backend foundation in this stack. Rephrase to avoid rollout ambiguity.
Suggested wording update
### Phase 3
- Prepare PostgreSQL-native backend compatibility
- Align repository contracts with future Go services
-- Introduce connection pooling and transactional services
+- Leverage existing connection pooling and introduce transactional services🤖 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 `@docs/migrations/postgres-repository-migration.md` around lines 53 - 56, The
Phase 3 bullet point "Introduce connection pooling and transactional services"
is misleading because connection pooling already exists in the current Go
backend foundation. Clarify this bullet point by rephrasing it to indicate that
Phase 3 is enhancing or optimizing the existing pooling for PostgreSQL-native
backend compatibility, rather than introducing pooling as a new feature. This
will eliminate ambiguity about whether pooling is being added for the first time
or is being adapted/upgraded as part of the migration.
| import { createClient } from '@supabase/supabase-js'; | ||
|
|
||
| const supabaseAdmin = createClient( | ||
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | ||
| process.env.SUPABASE_SERVICE_ROLE_KEY! | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify where this repository is imported.
rg -n --glob '**/*.{ts,tsx}' "from ['\"][^'\"]*credits-repository['\"]" -C2 || true
# For each importer, print whether it is a client module.
python - <<'PY'
from pathlib import Path
import re
pat = re.compile(r"from ['\"][^'\"]*credits-repository['\"]")
for p in list(Path('.').rglob('*.ts')) + list(Path('.').rglob('*.tsx')):
try:
txt = p.read_text(encoding='utf-8')
except Exception:
continue
if pat.search(txt):
head = "\n".join(txt.splitlines()[:8])
is_client = ("'use client'" in head) or ('"use client"' in head)
print(f"{p}: use_client={is_client}")
PYRepository: Muneerali199/Draftdeckai
Length of output: 434
🏁 Script executed:
# First, check the current state of the credits-repository file
head -20 lib/repositories/credits-repository.ts
# Then search exhaustively for ALL importers
echo "=== Searching for all imports of credits-repository ==="
rg -n "credits-repository" --glob '**/*.{ts,tsx,js,jsx}' || true
# Check if server-only is in package.json
echo "=== Checking for server-only in dependencies ==="
rg "server-only" package.json || echo "Not found in package.json"Repository: Muneerali199/Draftdeckai
Length of output: 766
🏁 Script executed:
# Get the full directory structure to understand project layout
echo "=== Project root files and directories ==="
ls -la
# Check if node_modules has server-only installed
ls -la node_modules/ | grep -i server || echo "Node modules check skipped or not found"Repository: Muneerali199/Draftdeckai
Length of output: 6612
🏁 Script executed:
# Search for any server-only usage in the codebase
echo "=== Checking for 'server-only' usage ==="
rg "server-only" --glob '**/*.{ts,tsx,js,jsx}' -C 1 || echo "No server-only imports found"
# Check the importing file to confirm it's server-side
echo "=== Contents of the importing file ==="
head -30 app/api/generate/resume/route.ts
# Check if there are other Supabase admin clients
echo "=== Checking for other Supabase admin clients ==="
rg "createClient.*SUPABASE.*KEY" --glob '**/*.{ts,tsx}' || true
# Check package.json to see if there are any related security packages
echo "=== Checking package.json for security-related packages ==="
grep -A 5 '"dependencies"' package.json | head -20Repository: Muneerali199/Draftdeckai
Length of output: 1709
🏁 Script executed:
# Verify the exact import statement and surrounding context
echo "=== Full import context in app/api/generate/resume/route.ts ==="
head -20 app/api/generate/resume/route.tsRepository: Muneerali199/Draftdeckai
Length of output: 915
Add a strict server-only guard for the admin Supabase client and consolidate client creation.
The supabaseAdmin client uses a service-role key without a server-only boundary, creating a runtime risk if accidentally imported client-side. Currently, only server-side code imports this module, but adding an explicit guard prevents future regressions.
Additionally, app/api/generate/resume/route.ts duplicates the client creation instead of importing the repository's export.
To implement the fix, add the server-only package to dependencies (it is not currently installed), then import it at the top of lib/repositories/credits-repository.ts to enforce server-only usage.
🤖 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 `@lib/repositories/credits-repository.ts` around lines 1 - 6, The supabaseAdmin
client creation lacks a server-only guard despite using a sensitive service-role
key, creating a risk of accidental client-side exposure. Add the server-only
package to your dependencies, then import it at the very top of
lib/repositories/credits-repository.ts before the createClient call for
supabaseAdmin. This will enforce that the module can only be imported on the
server, preventing future regressions if this module is accidentally referenced
in client-side code.
| const { data, error } = await supabaseAdmin | ||
| .from('user_credits') | ||
| .insert({ | ||
| user_id: userId, | ||
| tier: 'free', | ||
| credits_total: creditsTotal, | ||
| credits_used: 0, | ||
| credits_reset_at: resetDate, | ||
| }) | ||
| .select() | ||
| .single(); | ||
|
|
||
| if (error) { | ||
| throw new Error(`Failed to create user credits: ${error.message}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether user_credits has uniqueness on user_id in schema/migrations.
rg -n -i -C3 "create table|user_credits|unique|primary key|on conflict"
# Verify call patterns that may do get-then-create and therefore race.
rg -n --glob '**/*.{ts,tsx}' -C3 "getUserCredits\\(|createUserCredits\\("Repository: Muneerali199/Draftdeckai
Length of output: 50381
🏁 Script executed:
# Find and examine the credits-repository.ts file
fd -t f "credits-repository.ts" -x cat -n {} | head -100Repository: Muneerali199/Draftdeckai
Length of output: 2167
🏁 Script executed:
# Search for getUserCredits function definition
rg -n "getUserCredits" --type ts --type tsx -B2 -A8Repository: Muneerali199/Draftdeckai
Length of output: 96
🏁 Script executed:
# Look for call patterns that might race
rg -n "createUserCredits|getUserCredits" --type ts --type tsx -B3 -A3Repository: Muneerali199/Draftdeckai
Length of output: 96
🏁 Script executed:
# Search for where createUserCredits is called to identify race condition patterns
rg -n "createUserCredits" --type ts -B5 -A5Repository: Muneerali199/Draftdeckai
Length of output: 2348
🏁 Script executed:
# Also check for any onboarding or user creation flows
rg -n "onAuth|handleNewUser|signup" --type ts -B3 -A5 | head -80Repository: Muneerali199/Draftdeckai
Length of output: 5605
🏁 Script executed:
# Check Supabase error handling patterns in the codebase
rg -n "error\.code|error\.message" --type ts -B2 -A2 | head -60Repository: Muneerali199/Draftdeckai
Length of output: 3343
🏁 Script executed:
# Check all locations where createUserCredits is called
rg -n "createUserCredits" --type ts -B8 -A8Repository: Muneerali199/Draftdeckai
Length of output: 3545
🏁 Script executed:
# Check if there are other similar get-then-create patterns
rg -n "getUserCredits.*createUserCredits|if.*!.*userCredits" --type ts -B5 -A5Repository: Muneerali199/Draftdeckai
Length of output: 6134
🌐 Web query:
PostgreSQL error code 23505 unique constraint violation
💡 Result:
The PostgreSQL error code 23505, known as unique_violation, is a standard error that occurs when an INSERT or UPDATE operation attempts to violate a unique constraint [1][2][3]. This constraint is enforced by either a UNIQUE index or a PRIMARY KEY definition on a table column or a set of columns [2][4][5]. Common causes for this error include: 1. Inserting or updating a record with a value that already exists in a column defined as UNIQUE or PRIMARY KEY [2][4]. 2. Race conditions in concurrent applications where multiple transactions attempt to insert the same value simultaneously [2][6]. 3. Desynchronization of database sequences (often with SERIAL or IDENTITY columns), where the sequence counter falls behind the actual maximum value in the table [7][5]. Recommended approaches for handling or preventing this error include: - Use INSERT... ON CONFLICT (upsert): This is the preferred method for handling potential duplicates gracefully within a single statement [2][5]. You can specify either DO NOTHING to skip the insert or DO UPDATE to modify the existing record [2][5]. INSERT INTO your_table (column1, column2) VALUES ('value1', 'value2') ON CONFLICT (column1) DO NOTHING; - Application-level exception handling: If you cannot use an upsert, wrap your database operations in a try-catch block to specifically handle the 23505 error code [2][4]. Most database drivers provide a specific class or error code object (e.g., psycopg2.errors.UniqueViolation in Python) to catch this condition [2][5]. - Verify sequence synchronization: If you are seeing this error on a column using a SERIAL or IDENTITY type, check if the underlying sequence has fallen out of sync with the data. You can fix this by resetting the sequence using: SELECT SETVAL('your_table_id_seq'::regclass, (SELECT MAX(id) FROM your_table)); [7] The error message typically provides the name of the violated constraint, which can be used to identify exactly which column or index is causing the conflict [1][2][7].
Citations:
- 1: https://www.postgresql.org/docs/18/errcodes-appendix.html
- 2: https://stack-notes.com/postgres/23505
- 3: https://github.com/postgres/postgres/blob/207cb2ab/src/backend/utils/errcodes.txt
- 4: https://philipmcclarence.com/how-to-diagnose-and-fix-the-23505-unique_violation-error-code-in-postgres/
- 5: https://deadends.dev/database/unique-violation/
- 6: https://stackoverflow.com/questions/12385763/suppress-duplicate-key-value-violates-unique-constraint-errors
- 7: https://stackoverflow.com/questions/24390820/postgresql-error-23505-duplicate-key-value-violates-unique-constraint-foo-col
🏁 Script executed:
# Check how Supabase errors are handled elsewhere in the codebase
rg -n "error\.code|error\." lib/repositories --type ts -B2 -A2Repository: Muneerali199/Draftdeckai
Length of output: 981
🏁 Script executed:
# Check Supabase error structure in other files
rg -n "supabaseAdmin.*error" app/api --type ts -A5 | head -100Repository: Muneerali199/Draftdeckai
Length of output: 50
🌐 Web query:
Supabase JavaScript error code property unique constraint violation
💡 Result:
When a unique constraint violation occurs in Supabase, the JavaScript client returns a PostgrestError object with the specific error code '23505' [1][2][3]. This code corresponds to the PostgreSQL 'unique_violation' error [1][2]. You should programmatically branch on error.code rather than the error message, as the message can change between versions while the code remains stable [4]. To handle these violations, you can use the following approach in your application code [1]: const { data, error } = await supabase.from('your_table').insert({... }) if (error) { if (error.code === '23505') { // Handle unique constraint violation (e.g., show a user-friendly message) console.error('This record already exists.') } else { // Handle other errors console.error(error.message) } } In addition to the code, the error object includes a details field, which often specifies exactly which key or value caused the violation [4][3]. For common use cases where you want to avoid these errors (such as handling race conditions or form double-submissions), it is recommended to use the.upsert method instead of.insert [5][6]. When using.upsert, you can specify the onConflict option to target the unique column directly [6][3].
Citations:
- 1: https://www.rapidevelopers.com/supabase-tutorial/how-to-avoid-duplicated-entries-in-supabase
- 2: https://supabase.com/docs/guides/api/rest/postgrest-error-codes
- 3: supabase/supabase-js@c8a7734
- 4: https://supabase.com/docs/guides/api/handling-errors-in-supabase-js
- 5: Issue Description: Intermittent 409 Conflict on Insert with Unique Constraint supabase/supabase-js#2049
- 6: https://www.flowql.com/en/blog/guides/supabase-unique-constraint-error-fix/
Make createUserCredits idempotent to safely handle duplicate/concurrent requests.
The function at lines 37-51 throws on conflict instead of returning the existing record. When multiple requests race to create credits for a new user (or retries occur), the second attempt fails. The user_credits table has a UNIQUE constraint on user_id, making this pattern unsafe.
Catch error code 23505 (unique constraint violation) and fetch the existing record via getUserCredits(userId) instead of throwing. This prevents request failures when concurrent create attempts occur.
Suggested fix
export async function createUserCredits(
userId: string,
creditsTotal: number,
resetDate: string
): Promise<UserCredits> {
const { data, error } = await supabaseAdmin
.from('user_credits')
.insert({
user_id: userId,
tier: 'free',
credits_total: creditsTotal,
credits_used: 0,
credits_reset_at: resetDate,
})
.select()
.single();
if (error) {
+ // Duplicate create (e.g., race/retry) → return existing row instead of failing.
+ if (error.code === '23505') {
+ const existing = await getUserCredits(userId);
+ if (existing) return existing;
+ }
throw new Error(`Failed to create user credits: ${error.message}`);
}
return data;
}🤖 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 `@lib/repositories/credits-repository.ts` around lines 37 - 51, The
createUserCredits function throws an error when it encounters a unique
constraint violation (error code 23505) from concurrent requests, making it
non-idempotent. Modify the error handling logic to check if the error code is
23505 (unique constraint violation on the user_id field), and instead of
throwing, call the existing getUserCredits(userId) method to fetch and return
the existing record. This allows the function to safely handle duplicate or
concurrent creation attempts without failing on subsequent requests.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 33127064 | Triggered | Generic Password | 48fd85c | tests/lib/validation.test.ts | View secret |
| 33127065 | Triggered | Generic Password | 48fd85c | tests/lib/validation.test.ts | View secret |
| 33127066 | Triggered | Generic Password | 48fd85c | tests/lib/validation.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
7 participants
##program-gssoc26
Description
This PR adds a PostgreSQL/Supabase database layer to the Go backend using
pgx/v5and its connection pooling helperpgxpool. This allows migrating endpoints from Next.js to Go to query and persist data, replacing the stub server implementation.Fixes #893
Type of Change
Changes Made
github.com/jackc/pgx/v5 v5.6.0tobackend/go.mod.backend/go/internal/database/database.gowrapping*pgxpool.Poolfor repositories.backend/go/internal/database/pool.goimplementing:DATABASE_URL,DB_MAX_CONNS,DB_MIN_CONNS,DB_MAX_CONN_LIFETIME,DB_MAX_CONN_IDLE_TIME).backend/go/cmd/server/main.goto validate presence ofDATABASE_URL, initialize the pool on startup, and drain all connections in the pool on graceful shutdown..env.examplewith the new environment variables and defaults.pool_test.goto test database config loading, default values, and overrides.Dependencies
github.com/jackc/pgx/v5 v5.6.0Checklist
Summary by CodeRabbit
New Features
Chores