diff --git a/.env.example b/.env.example index 5fdee7d..2da1844 100644 --- a/.env.example +++ b/.env.example @@ -1,7 +1,7 @@ # App Config APP_PORT=8081 ENV=development -JWT_SECRET_KEY=DoormileSuperSecretJWTKey2026! +JWT_SECRET_KEY=change-me-locally INTERNAL_API_KEY=doormile-internal-2024 # Reverse proxy — comma-separated IPs/CIDRs allowed to set X-Forwarded-For. @@ -16,13 +16,13 @@ DB_HOST=31.97.228.132 DB_PORT=5433 DB_NAME=logistics DB_USER=admin -DB_PASSWORD=Package@321# +DB_PASSWORD= # Redis Configuration REDIS_HOST=31.97.228.132 REDIS_PORT=6379 REDIS_USER=admin -REDIS_PASSWORD=Package@321# +REDIS_PASSWORD= # SMTP Configuration (email OTP verification) SMTP_HOST=smtp.gmail.com @@ -38,4 +38,26 @@ $env:PATH += ";C:\Program Files\Docker\Docker\resources\bin" >> docker push doormile/doormile-backend:latest >> - +# ── Required / changed 2026-09-11 ─────────────────────────────────────────── +# JWT_SECRET_KEY no longer has a default. It used to fall back to a literal in +# config/config.go, which meant anyone holding this repository could mint a +# valid token for any user id and any role against a deployment that had not +# overridden it. +# ENV=production + unset -> the service REFUSES TO START (cfg.Validate). +# anything else + unset -> an ephemeral per-process key is generated and a +# warning logged; tokens will not survive a restart. +# Set it for a stable local session, and make sure it is set in production +# before deploying (the JWT_SECRET_KEY line above). +# +# NATS_URL and the AI/optimiser hosts also lost their defaults, which pointed at +# the real production cluster — an unconfigured local run silently joined the +# live stream and competed with the production workers. Unset now means +# "disabled": no NATS connection, no route sequencing, legacy assignment +# scoring. Set them explicitly where you actually want them. +# NATS_URL=nats://localhost:4222 +# NATS_USER= +# NATS_PASSWORD= +# AI_LAYER_BASE_URL= +# ROUTE_OPTIMIZER_URL= +# +# DB_PASSWORD has no default either — set it for your own database. diff --git a/CLAUDE.md b/CLAUDE.md index e4bb875..68fdeb0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -237,8 +237,11 @@ websocket routes. **[verified this session]** confirm current values rather than assuming): admin login at `suriya@doormile.com`; hub staff accounts pattern `hub.[city]@doormile.in` plus partner variants; miler test phone numbers + PINs. -- Auth: Firebase OTP for customer login — this cannot be scripted/bypassed - from the command line, which is why the last live E2E test stalled (§9). +- Auth: **not Firebase.** Customer login is a 4-digit OTP to phone or email, + issued by `issueCxOtp` and stored in Redis (§8.5). `CX_STAGING_OTP` makes it + a fixed code outside production, which is what lets it be scripted — the + earlier "cannot be bypassed from the command line" note (§9) predates that + and predates the `/customer/*` rebuild. --- @@ -597,6 +600,137 @@ key, after the legacy rider app's key had to be revoked), `MILER_CALL_PROXY` --- +## 8.6 Customer sign-in, config hardening & the ordering fix (2026-09-11) + +**[verified this session]** — six defects were reported against customer +bookings. Five were real; one was not. Everything below was verified against +the code, and the schema question against the **production database**. + +### The one that locked every customer out (DM-01) + +`CxVerifyOtp` read `json:"code"`. The customer app sent `otp` — because +`docs/customer-app-api-crisp.md` documented `otp`, while +`docs/openapi-customer.yaml` correctly said `code`. **The two docs disagreed +and the app was built from the wrong one.** `req.Code` was therefore always +empty, the empty-code branch always fired, and every sign-in failed with +`400 "Enter the code we sent you"` — a correct code failed exactly like a +wrong one. No amount of SMS-gateway credit would have fixed it. + +The handler now accepts `otp` as a **deprecated alias**; `code` wins when both +are present. This deliberately fixes builds already in customers' hands, which +an app-side fix alone cannot. `controllers/cxOtpFieldAlias_test.go` pins both +names, the precedence, and that a blank/missing code is still rejected. Remove +the alias once the install base has moved on. + +### A failed OTP send no longer punishes the customer (DM-05) + +`issueCxOtp` writes the code, the resend cooldown and the rate-limit slot +*before* attempting delivery — it has to, the code must exist to be sent. But +on failure it kept all three: the customer was told something went wrong, could +not resend until the cooldown expired, had spent one of their five hourly +codes, and a valid code they never received sat live in Redis for its full TTL. +All three are now rolled back on a delivery error (`DEL` the code and cooldown, +`DECR` the rate counter), so a retry is immediate and nothing usable is left. + +### Production credentials are no longer defaults (DM-06) + +`config.Load()` defaulted `JWT_SECRET_KEY` to a literal, and `NATS_URL` / +`NATS_USER` / `NATS_PASSWORD` / `AI_LAYER_BASE_URL` / `ROUTE_OPTIMIZER_URL` / +`DB_PASSWORD` to the real production values. Two consequences, both live: +a clone of this repository could mint a valid token for any user id and any +role against any deployment that had not overridden the secret; and `go run .` +on a laptop silently joined the production NATS cluster and competed with the +real workers for the same durable consumer. **This was hit accidentally on +2026-09-11** — a local instance pulled `api.v1.bookings.update` for real +booking ids and caused redelivery churn on production for ~40 seconds. + +All now default to empty, and the empty case is handled rather than assumed: +`InitNATS` skips connecting, `routing.BaseURL == ""` already disabled +sequencing, and the AI layer returns an error so the caller's existing +`AI_LAYER_FALLBACK` path takes over with legacy scoring. A second hardcoded +production URL in `internal/assignment/ai_layer.go` (not in the original +report) was removed too. + +`JWT_SECRET_KEY` is special-cased because an empty signing key is worse than a +shared one: **`cfg.Validate()`, called from `main`, refuses to start** when it +is unset in production. Outside production an ephemeral per-process key is +generated with a warning, so local development needs no configuration while +tokens stop surviving a restart. `GEOCODER_URL` deliberately keeps its default +— Nominatim is a public service, not a Doormile host. + +### `GET /admin/bookings` is ordered (DM-04) + +Added `Order("bookingid DESC")`. Without it the row order was unspecified — +Postgres heap order, oldest first — which put the newest booking on the LAST +page, outside the console's bounded drain window, and made `OFFSET` paging +unstable enough to duplicate and skip rows. The primary key is unique, so the +sort needs no tiebreaker. + +The console half lives in the admin console repo (`krow_talent_app` — the +directory name is stale; it is the Doormile Express Console): it requests +`pagesize=1000`, receives 100, and stops after 12 pages, so it sees 1200 rows +regardless. With the list now newest-first those 1200 are the most recent ones +rather than the oldest, which turns a silent disappearance into a bounded view. + +### DM-03 (timestamp drift) is NOT a production bug — do not "fix" it + +Reported as `DBNow()` relabelling IST wall-clock as UTC against +`timestamp with time zone` columns, causing a +5:30 drift that hid evening +bookings from the console. **Verified against production and it is false +there:** + +``` +pickupbookings.createdat timestamp without time zone +pickupbookings.preferredpickupfrom timestamp without time zone +appcustomers.createdat timestamp without time zone +``` + +Which is exactly what `DBNow()`'s own comment assumes. A round-trip confirmed +it: a customer created at a known `12:10:34 IST` stored as `12:10:34.094323`. +Zero drift. **Changing `DBNow()` would introduce the bug, not fix it.** + +The real finding is the reporter's own fallback: GORM's Postgres driver maps +`time.Time` to `timestamptz`, so a schema built fresh from `AutoMigrate` does +NOT match production, and every new dev environment WILL show the +5:30 drift +that production does not. That is why they saw it. Pin the column types +explicitly in the models before this bites someone again. + +### Docs corrected + +`docs/customer-app-api-crisp.md` had **three** request shapes that did not +match their parsers, all failing silently through `BodyParser` — no error, just +a zero value: + +| Endpoint | Documented | Actually parsed | +|---|---|---| +| `auth/otp/verify` | `otp` | `code` (now both) | +| `fare/estimate` | `pickup.latitude`/`longitude`, `packages[].weightKg` | `pickup.lat`/`lng`, `packageCount` | +| `bookings` | flat destination fields, `pickup.latitude` | nested `details{}`, `pickup.lat`/`lng` | + +The booking **response** block was wrong the same way (`latitude`/`longitude` +where `renderCxBooking` emits `lat`/`lng`). `docs/express-console-api.md` also +claimed pagination "default 500, cap 1000" when the code is default 20, cap 100 +— which is what made the console size its page budget for twelve times the rows +it actually receives. + +**When a doc and a parser disagree here, the parser has won every time.** Three +separate client teams have now built against wrong Doormile docs in one week. + +### Still open from this report + +- **DM-02: email OTP returns 500 on production.** `SMTP_HOST`, `SMTP_USER` and + `SMTP_PASSWORD` all default to `""` and are not set. Either configure SMTP or + hide the app's Email tab — offering a path that always fails is worse than + not offering it. +- The **committed secrets** (`.env` and a live GCP service-account private key) + are still tracked in git and pushed. Removing the defaults above does not + help until those keys are **rotated**. +- `GET /api/v1/ready` returns **503 while its body says `"status":"ready"`** + (`routes/routes.go`) — a monitor reading the body sees the opposite of the + status code. + +--- + ## 9. Current blockers & open work (whole-project level) **[carried forward]** diff --git a/cmd/migrate_qdrant/main.go b/cmd/migrate_qdrant/main.go index 67ccc8c..11cb0a8 100644 --- a/cmd/migrate_qdrant/main.go +++ b/cmd/migrate_qdrant/main.go @@ -35,7 +35,11 @@ func main() { "host=%s user=%s password=%s dbname=%s port=%s sslmode=disable TimeZone=Asia/Kolkata", getenv("DB_HOST", "127.0.0.1"), getenv("DB_USER", "admin"), - getenv("DB_PASSWORD", "Package@321#"), + // No default: this used to carry the live database password, so the + // credential shipped with every clone of the repository. Unset means + // unset — the connection will fail with a clear error instead of + // silently reaching production. + getenv("DB_PASSWORD", ""), getenv("DB_NAME", "logistics"), getenv("DB_PORT", "5433"), ) diff --git a/config/config.go b/config/config.go index d5b6120..f224837 100644 --- a/config/config.go +++ b/config/config.go @@ -1,7 +1,13 @@ package config import ( + "crypto/rand" + "encoding/hex" + "fmt" "os" + "strings" + + "doormile/utils" ) type Config struct { @@ -52,25 +58,93 @@ type Config struct { } func Load() *Config { - return &Config{ - Env: getEnv("ENV", "development"), - Port: getEnv("APP_PORT", "8081"), - DBName: getEnv("DB_NAME", "logistics"), - DBUser: getEnv("DB_USER", "admin"), - DBPassword: getEnv("DB_PASSWORD", "Package@321#"), - DBPort: getEnv("DB_PORT", "5433"), - DBHost: getEnv("DB_HOST", "127.0.0.1"), - RedisHost: getEnv("REDIS_HOST", "127.0.0.1"), - RedisPort: getEnv("REDIS_PORT", "6379"), - RedisUser: getEnv("REDIS_USER", ""), - RedisPassword: getEnv("REDIS_PASSWORD", ""), - JWTSecret: getEnv("JWT_SECRET_KEY", "DoormileSuperSecretJWTKey2026!"), - NatsURL: getEnv("NATS_URL", "nats://66.116.226.161:4223"), - NatsUser: getEnv("NATS_USER", "doormile"), - NatsPassword: getEnv("NATS_PASSWORD", "Package@321#"), - AILayerBaseURL: getEnv("AI_LAYER_BASE_URL", "https://routemate.workolik.com"), + cfg := load() + cfg.hardenSecrets() + return cfg +} - RouteOptimizerURL: getEnv("ROUTE_OPTIMIZER_URL", "https://routes.workolik.com"), +// IsProduction reports whether this process is running as production. Used by +// the guards that must behave differently there — a fixed OTP, a missing JWT +// secret — rather than scattering string comparisons. +func (c *Config) IsProduction() bool { + return strings.EqualFold(strings.TrimSpace(c.Env), "production") +} + +// hardenSecrets refuses to let the service run on a guessable signing key. +// +// JWT_SECRET_KEY used to default to a literal in this file. Anyone holding the +// repository could mint a token for any user id and any role, against any +// deployment that had not overridden it — which is the whole authorisation +// model, given away by a git clone. +// +// In production an unset secret is fatal: booting with a known key is worse +// than not booting, because nothing external shows that anything is wrong. +// Anywhere else it becomes a random per-process key, so local development +// works without configuration while tokens stop surviving a restart and can +// never be valid anywhere but this process. +func (c *Config) hardenSecrets() { + if strings.TrimSpace(c.JWTSecret) != "" { + return + } + // In production an absent secret is left absent, so Validate can refuse the + // boot with a clear message. Generating one here would be worse than the + // old default: every restart would invalidate every live session, and + // nothing would say why. + if c.IsProduction() { + return + } + b := make([]byte, 32) + if _, err := rand.Read(b); err != nil { + // Leave it empty; Validate turns this into a refusal to start. + return + } + c.JWTSecret = hex.EncodeToString(b) + utils.Warn("JWT_SECRET_KEY is not set — generated an ephemeral key for this process only. " + + "Tokens will not survive a restart. Set JWT_SECRET_KEY for a stable local session.") +} + +// Validate reports configuration that must prevent the service from starting. +// Called by main; kept separate from Load so that loading stays free of side +// effects and the package remains testable. +func (c *Config) Validate() error { + if strings.TrimSpace(c.JWTSecret) == "" { + return fmt.Errorf("JWT_SECRET_KEY is not set (ENV=%s): refusing to start, because "+ + "booting on a default or empty signing key lets anyone holding this repository "+ + "mint a valid token for any account", c.Env) + } + return nil +} + +func load() *Config { + return &Config{ + Env: getEnv("ENV", "development"), + Port: getEnv("APP_PORT", "8081"), + DBName: getEnv("DB_NAME", "logistics"), + DBUser: getEnv("DB_USER", "admin"), + DBPassword: getEnv("DB_PASSWORD", ""), + DBPort: getEnv("DB_PORT", "5433"), + DBHost: getEnv("DB_HOST", "127.0.0.1"), + RedisHost: getEnv("REDIS_HOST", "127.0.0.1"), + RedisPort: getEnv("REDIS_PORT", "6379"), + RedisUser: getEnv("REDIS_USER", ""), + RedisPassword: getEnv("REDIS_PASSWORD", ""), + // No default. See hardenSecrets below — an unset secret is either a + // refusal to boot or an ephemeral per-process key, never a shared one + // baked into the source. + JWTSecret: getEnv("JWT_SECRET_KEY", ""), + + // These defaulted to the real production hosts and credentials, which + // meant `go run .` on a laptop silently joined the live NATS stream and + // competed with the production workers for the same durable consumer. + // Empty now: InitNATS skips connecting, routing.BaseURL == "" disables + // sequencing, and the AI layer falls back to legacy scoring. Fail + // closed, so reaching production is something you opt into. + NatsURL: getEnv("NATS_URL", ""), + NatsUser: getEnv("NATS_USER", ""), + NatsPassword: getEnv("NATS_PASSWORD", ""), + AILayerBaseURL: getEnv("AI_LAYER_BASE_URL", ""), + + RouteOptimizerURL: getEnv("ROUTE_OPTIMIZER_URL", ""), GeocoderURL: getEnv("GEOCODER_URL", "https://nominatim.openstreetmap.org"), GeocoderEmail: getEnv("GEOCODER_EMAIL", ""), TrustedProxies: getEnv("TRUSTED_PROXIES", ""), diff --git a/config/secrets_test.go b/config/secrets_test.go new file mode 100644 index 0000000..a2b356c --- /dev/null +++ b/config/secrets_test.go @@ -0,0 +1,142 @@ +package config + +import ( + "strings" + "testing" +) + +// DM-06: config.go used to default JWT_SECRET_KEY, the NATS URL and its +// credentials, and the AI/optimiser hosts to the REAL production values. Two +// consequences: anyone holding the repository could mint a valid token for any +// account against a deployment that had not overridden the secret, and any +// local run silently joined the live NATS stream. + +// The literals that must never come back. Written out so a revert is a test +// failure rather than something noticed in review. +func TestProductionValuesAreNotDefaults(t *testing.T) { + for _, key := range []string{ + "JWT_SECRET_KEY", "NATS_URL", "NATS_USER", "NATS_PASSWORD", + "AI_LAYER_BASE_URL", "ROUTE_OPTIMIZER_URL", "DB_PASSWORD", + } { + setEnv(t, key, "") + } + setEnv(t, "ENV", "development") + + cfg := Load() + + banned := map[string]string{ + "NatsURL": cfg.NatsURL, + "NatsUser": cfg.NatsUser, + "NatsPassword": cfg.NatsPassword, + "AILayerBaseURL": cfg.AILayerBaseURL, + "RouteOptimizerURL": cfg.RouteOptimizerURL, + "DBPassword": cfg.DBPassword, + } + for field, got := range banned { + if got != "" { + t.Errorf("%s defaulted to %q — production values must not be defaults", field, got) + } + } + + // The old hardcoded secret must not be what we sign with. + if cfg.JWTSecret == "DoormileSuperSecretJWTKey2026!" { + t.Error("JWTSecret fell back to the literal that used to be in config.go") + } +} + +// With no secret configured outside production the service still runs, but on +// a key that exists only for this process. +func TestUnsetSecretOutsideProductionIsEphemeralNotShared(t *testing.T) { + setEnv(t, "JWT_SECRET_KEY", "") + setEnv(t, "ENV", "development") + + first := Load().JWTSecret + second := Load().JWTSecret + + if first == "" || second == "" { + t.Fatal("an unset secret produced an empty signing key; tokens would be forgeable") + } + if first == second { + t.Error("two loads produced the same generated key — it is not ephemeral") + } + if len(first) < 32 { + t.Errorf("generated key is %d chars, too short to be a signing key", len(first)) + } +} + +// In production an absent secret is NOT quietly replaced — it is left absent so +// Validate can refuse the boot with a message that says why. Silently +// generating one would invalidate every live session on each restart. +func TestProductionRefusesToStartWithoutASecret(t *testing.T) { + setEnv(t, "JWT_SECRET_KEY", "") + setEnv(t, "ENV", "production") + + cfg := Load() + if cfg.JWTSecret != "" { + t.Errorf("production generated a secret (%q); it must stay empty so Validate fails", cfg.JWTSecret) + } + err := cfg.Validate() + if err == nil { + t.Fatal("Validate accepted an empty JWT secret in production") + } + if !strings.Contains(err.Error(), "JWT_SECRET_KEY") { + t.Errorf("Validate error does not name the variable: %v", err) + } +} + +// Outside production the ephemeral key is enough to pass validation, so local +// development needs no configuration at all. +func TestValidatePassesOutsideProductionWithNoSecret(t *testing.T) { + setEnv(t, "JWT_SECRET_KEY", "") + setEnv(t, "ENV", "development") + + if err := Load().Validate(); err != nil { + t.Errorf("development should boot without a configured secret: %v", err) + } +} + +// A configured secret always validates, production or not. +func TestValidatePassesWithAConfiguredSecret(t *testing.T) { + setEnv(t, "JWT_SECRET_KEY", "a-real-configured-secret") + setEnv(t, "ENV", "production") + + if err := Load().Validate(); err != nil { + t.Errorf("a configured secret must validate: %v", err) + } +} + +// An explicitly configured secret is always used verbatim. +func TestConfiguredSecretIsUsedVerbatim(t *testing.T) { + setEnv(t, "JWT_SECRET_KEY", "a-real-configured-secret") + setEnv(t, "ENV", "production") + + if got := Load().JWTSecret; got != "a-real-configured-secret" { + t.Errorf("JWTSecret = %q, want the configured value", got) + } +} + +func TestIsProductionIsCaseAndSpaceInsensitive(t *testing.T) { + setEnv(t, "JWT_SECRET_KEY", "x") + for _, env := range []string{"production", "Production", "PRODUCTION", " production "} { + setEnv(t, "ENV", env) + if !Load().IsProduction() { + t.Errorf("ENV=%q was not treated as production", env) + } + } + for _, env := range []string{"development", "staging", "test", ""} { + setEnv(t, "ENV", env) + if Load().IsProduction() { + t.Errorf("ENV=%q was treated as production", env) + } + } +} + +// The geocoder is a PUBLIC service, not a Doormile host, so it keeps its +// default — removing it would break place search for no security gain. +func TestGeocoderKeepsItsPublicDefault(t *testing.T) { + setEnv(t, "JWT_SECRET_KEY", "x") + setEnv(t, "GEOCODER_URL", "") + if got := Load().GeocoderURL; !strings.Contains(got, "nominatim") { + t.Errorf("GeocoderURL = %q, want the public Nominatim default", got) + } +} diff --git a/controllers/cxAuthController.go b/controllers/cxAuthController.go index 9fa56cd..c8f948d 100644 --- a/controllers/cxAuthController.go +++ b/controllers/cxAuthController.go @@ -201,14 +201,38 @@ func issueCxOtp(cfg *config.Config, identifier, kind string) (resendAfter int, e db.Rdb.Del(ctx, cxOtpTriesKey(identifier)) db.Rdb.Set(ctx, cxOtpSentKey(identifier), "1", cxResendWait) + // A code that was never delivered must leave nothing behind. + // + // The stored code, the resend cooldown and the rate-limit slot are all + // written BEFORE delivery is attempted, because they have to be — the code + // has to exist before it can be sent. But when sending fails, keeping them + // punishes the customer for the gateway's failure: they are told something + // went wrong, cannot resend until the cooldown expires, and have spent one + // of their five hourly codes — while a valid code they never received sits + // live in Redis for its full TTL. + // + // So unwind all three on failure. The customer can retry immediately, and + // nothing usable is left in Redis. + rollback := func() { + rctx, rcancel := context.WithTimeout(context.Background(), 3*time.Second) + defer rcancel() + db.Rdb.Del(rctx, cxOtpKey(identifier)) + db.Rdb.Del(rctx, cxOtpSentKey(identifier)) + // Give the slot back rather than deleting the window: DECR keeps the + // hourly window honest for codes that DID go out. + db.Rdb.Decr(rctx, cxOtpRateKey(identifier)) + } + if kind == "email" { if merr := mail.SendOTPEmail(cfg, identifier, code); merr != nil { - utils.Warn("cx auth: failed to send OTP email", "error", merr) + utils.Warn("cx auth: failed to send OTP email — rolling back the stored code", "error", merr) + rollback() return 0, merr } } else { if serr := sms.SendOTP(identifier, code); serr != nil { - utils.Warn("cx auth: failed to send OTP sms", "error", serr) + utils.Warn("cx auth: failed to send OTP sms — rolling back the stored code", "error", serr) + rollback() return 0, serr } } @@ -368,18 +392,37 @@ func CxVerifyOtp(cfg *config.Config) fiber.Handler { var req struct { Identifier string `json:"identifier"` Code string `json:"code"` - Name string `json:"name"` + // Otp is a DEPRECATED alias for Code, and the only reason sign-in + // works for anyone on an already-installed build. + // + // The customer app was written against a spec that named this + // field "otp". The server only ever read "code", so req.Code was + // always empty, the empty-code branch below always fired, and + // EVERY sign-in failed with a 400 — a correct code failed exactly + // like a wrong one. Fixing the app alone would have left every + // customer locked out until they updated; accepting both keys + // fixes them all without a release. + // + // "code" stays the documented field. Remove this once the install + // base has moved on. + Otp string `json:"otp"` + Name string `json:"name"` } if err := c.BodyParser(&req); err != nil { return utils.CxBadRequest(c, "We could not read that request") } + code := strings.TrimSpace(req.Code) + if code == "" { + code = strings.TrimSpace(req.Otp) + } + identifier, kind, ok := normalizeIdentifier(req.Identifier) - if !ok || strings.TrimSpace(req.Code) == "" { + if !ok || code == "" { return utils.CxBadRequest(c, "Enter the code we sent you") } - if !consumeCxOtp(identifier, strings.TrimSpace(req.Code)) { + if !consumeCxOtp(identifier, code) { return utils.CxFail(c, fiber.StatusUnauthorized, utils.CxErrInvalidOtp, "That code did not match") } diff --git a/controllers/cxOtpFieldAlias_test.go b/controllers/cxOtpFieldAlias_test.go new file mode 100644 index 0000000..385e899 --- /dev/null +++ b/controllers/cxOtpFieldAlias_test.go @@ -0,0 +1,88 @@ +package controllers + +import ( + "encoding/json" + "strings" + "testing" +) + +// DM-01: the customer app posts the verification code as "otp"; the server was +// written to read "code". req.Code was therefore always empty and EVERY +// sign-in failed with a 400 — a correct code failed exactly like a wrong one. +// +// These pin the alias. The struct is re-declared here to match the handler's +// anonymous one; what is under test is that both wire names reach the same +// value and that the precedence is stable. +type cxVerifyBody struct { + Identifier string `json:"identifier"` + Code string `json:"code"` + Otp string `json:"otp"` + Name string `json:"name"` +} + +// codeFrom mirrors the handler's selection: Code wins, Otp is the fallback. +func codeFrom(b cxVerifyBody) string { + code := strings.TrimSpace(b.Code) + if code == "" { + code = strings.TrimSpace(b.Otp) + } + return code +} + +func parseVerify(t *testing.T, raw string) cxVerifyBody { + t.Helper() + var b cxVerifyBody + if err := json.Unmarshal([]byte(raw), &b); err != nil { + t.Fatalf("unmarshal %s: %v", raw, err) + } + return b +} + +// The shape the app actually sends. This is the regression that locked every +// customer out of production. +func TestVerifyAcceptsTheAppsOtpField(t *testing.T) { + body := parseVerify(t, `{"identifier":"+919000000001","otp":"123456"}`) + if got := codeFrom(body); got != "123456" { + t.Errorf(`{"otp":"123456"} yielded %q — the app's field is being dropped again`, got) + } +} + +// The documented field keeps working unchanged. +func TestVerifyStillAcceptsCode(t *testing.T) { + body := parseVerify(t, `{"identifier":"+919000000001","code":"123456"}`) + if got := codeFrom(body); got != "123456" { + t.Errorf(`{"code":"123456"} yielded %q, want "123456"`, got) + } +} + +// When a client sends both, the documented field wins — so "code" stays the +// contract and "otp" can be removed later without changing behaviour for +// anyone who migrated. +func TestCodeWinsOverOtpWhenBothArePresent(t *testing.T) { + body := parseVerify(t, `{"identifier":"+919000000001","code":"111111","otp":"222222"}`) + if got := codeFrom(body); got != "111111" { + t.Errorf("got %q, want the documented `code` value 111111", got) + } +} + +// Neither field, or whitespace only, is still the empty-code rejection. The +// alias must not turn a missing code into an accepted one. +func TestMissingOrBlankCodeIsStillRejected(t *testing.T) { + for _, raw := range []string{ + `{"identifier":"+919000000001"}`, + `{"identifier":"+919000000001","code":"","otp":""}`, + `{"identifier":"+919000000001","code":" "}`, + `{"identifier":"+919000000001","otp":" "}`, + } { + if got := codeFrom(parseVerify(t, raw)); got != "" { + t.Errorf("%s yielded %q, want empty so the handler rejects it", raw, got) + } + } +} + +// A code arriving with padding must still match the stored one. +func TestPaddedOtpIsTrimmed(t *testing.T) { + if got := codeFrom(parseVerify(t, `{"identifier":"x","otp":" 123456 "}`)); got != "123456" { + t.Errorf("got %q, want the trimmed 123456", got) + } +} diff --git a/db/connect.go b/db/connect.go index a7092e8..bf8b337 100644 --- a/db/connect.go +++ b/db/connect.go @@ -5,6 +5,7 @@ import ( "database/sql" "fmt" "os" + "strings" "time" "doormile/config" @@ -142,6 +143,17 @@ func InitRedis(cfg *config.Config) { } func InitNATS(cfg *config.Config) { + // No URL means NATS is deliberately not configured, so do not connect. + // This used to default to the production cluster, which meant any local + // run joined the live stream and competed with the real workers for the + // same durable pull consumer — messages got redelivered rather than lost, + // but it was production churn caused by someone running the repo. + if strings.TrimSpace(cfg.NatsURL) == "" { + utils.Info("NATS_URL is not set — running without NATS. " + + "Publishes are dropped and no consumer is started.") + return + } + var err error Nc, err = nats.Connect( cfg.NatsURL, diff --git a/db/nats_guard_test.go b/db/nats_guard_test.go new file mode 100644 index 0000000..47ff3d6 --- /dev/null +++ b/db/nats_guard_test.go @@ -0,0 +1,50 @@ +package db + +import ( + "testing" + + "doormile/config" +) + +// DM-06: NATS_URL used to default to the production cluster, so a backend run +// locally with no NATS configuration silently joined the live stream and +// competed with the production workers for the same durable pull consumer. +// That happened for real on 2026-09-11. +// +// The default is now empty, and empty must mean "do not connect" rather than +// "connect to whatever nats.Connect does with an empty string" — which would +// be localhost:4222, i.e. still a connection attempt. +func TestInitNATSSkipsWhenNoURLIsConfigured(t *testing.T) { + previousNc, previousJs := Nc, Js + t.Cleanup(func() { Nc, Js = previousNc, previousJs }) + Nc, Js = nil, nil + + for _, url := range []string{"", " "} { + Nc, Js = nil, nil + InitNATS(&config.Config{NatsURL: url}) + if Nc != nil { + t.Errorf("NatsURL=%q opened a connection; empty must mean no NATS", url) + Nc.Close() + Nc = nil + } + if Js != nil { + t.Errorf("NatsURL=%q initialised JetStream; empty must mean no NATS", url) + } + } +} + +// The config side of the same guarantee: no NATS setting may carry a real +// default, or the guard above is bypassed before it is ever reached. +func TestNATSConfigHasNoProductionDefaults(t *testing.T) { + for _, key := range []string{"NATS_URL", "NATS_USER", "NATS_PASSWORD"} { + t.Setenv(key, "") + } + t.Setenv("JWT_SECRET_KEY", "test") + t.Setenv("ENV", "development") + + cfg := config.Load() + if cfg.NatsURL != "" || cfg.NatsUser != "" || cfg.NatsPassword != "" { + t.Errorf("NATS settings defaulted to %q / %q / %q — all must be empty", + cfg.NatsURL, cfg.NatsUser, cfg.NatsPassword) + } +} diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index cbe9996..0aad410 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,73 @@ This document provides a comprehensive log of the major features, architectural --- +## 0. Customer sign-in fix, config hardening & admin ordering (2026-09-11) + +Six defects were reported against customer bookings. Five were real, one was +not. Full detail in `CLAUDE.md` §8.6. + +**Fixed** + +- **`POST /customer/auth/otp/verify` now accepts `otp` as well as `code`.** + The handler only ever read `code`; the app sent `otp`, because this repo's + own quick-reference documented `otp` while `openapi-customer.yaml` said + `code`. Every sign-in failed with a `400` — a correct code failed exactly + like a wrong one. `code` remains the contract and wins when both are sent; + `otp` is a deprecated alias kept so builds already installed keep working. +- **A failed OTP send no longer leaves a live code behind.** `issueCxOtp` now + rolls back the stored code, the resend cooldown and the rate-limit slot when + SMS or email delivery fails, instead of charging the customer for the + gateway's failure. +- **`GET /admin/bookings` is ordered `bookingid DESC`.** It had no `ORDER BY`, + so row order was unspecified — in practice oldest first, putting the newest + booking on the last page and outside any client that reads a bounded number + of pages. `OFFSET` paging over an unordered result was also unstable. +- **Production hosts and credentials removed from `config.Load()` defaults.** + `JWT_SECRET_KEY`, `NATS_URL`/`NATS_USER`/`NATS_PASSWORD`, + `AI_LAYER_BASE_URL`, `ROUTE_OPTIMIZER_URL` and `DB_PASSWORD` all defaulted to + real values, so a clone of this repo could mint a valid token for any account + and any local run joined the live NATS stream. A second hardcoded production + URL in `internal/assignment/ai_layer.go` was removed too. + +**Behaviour change to be aware of when deploying** + +`cfg.Validate()` now runs at startup and **the service refuses to boot when +`JWT_SECRET_KEY` is unset and `ENV=production`**. Outside production an +ephemeral per-process key is generated with a warning, so local development +needs no configuration — but tokens no longer survive a restart unless you set +the variable. Make sure `JWT_SECRET_KEY` is present in the production +environment before the next deploy. + +Unset `NATS_URL` now means "no NATS" rather than "production NATS": publishes +are dropped and no consumer starts. Set it explicitly wherever NATS is wanted. + +**Investigated and rejected** + +A reported +5:30 timestamp drift (`DBNow()` vs `timestamp with time zone` +columns) does **not** exist on production — the columns there are `timestamp +without time zone`, which is what `DBNow()` assumes, confirmed by a round-trip +with zero drift. Changing `DBNow()` would introduce the bug. The real risk is +that GORM's `AutoMigrate` produces `timestamptz`, so a freshly built schema +does not match production and every new dev environment shows a drift that +production does not. + +**Docs corrected** + +`customer-app-api-crisp.md` carried three request shapes that did not match +their parsers (`auth/otp/verify`, `fare/estimate`, `bookings`) plus a wrong +booking response shape; `express-console-api.md` documented pagination as +"default 500, cap 1000" when the code enforces default 20, cap 100. Every one +of these failed silently through `BodyParser` or a page budget, never as an +error. + +**Still open** + +Email OTP returns 500 on production (`SMTP_*` unset); `.env` and a live GCP +service-account key remain committed and need rotating; `GET /api/v1/ready` +returns 503 with a body that says `"status":"ready"`. + +--- + ## 1. Customer App v1 API Rebuild (`doormile_cx`) The customer-facing surface was completely rebuilt from the legacy single-destination / PIN-based flow to the production Customer App v1 contract. diff --git a/docs/customer-app-api-crisp.md b/docs/customer-app-api-crisp.md index 86335a6..25713e2 100644 --- a/docs/customer-app-api-crisp.md +++ b/docs/customer-app-api-crisp.md @@ -84,11 +84,22 @@ ## 3. Core Request & Response Payloads ### 1) OTP Verification (`POST /customer/auth/otp/verify`) + +> **The field is `code`, not `otp`.** This section said `otp` until 11 Sep 2026 +> and the customer app was built against it, while the server only ever read +> `code`. Every sign-in therefore failed with a `400 "Enter the code we sent +> you"` — a correct code failed exactly like a wrong one. `openapi-customer.yaml` +> had it right all along; the two disagreed and this one was wrong. +> +> The server now also accepts `otp` as a **deprecated alias**, so builds already +> in customers' hands keep working. Send `code`. If both are present, `code` +> wins. + ```json // Request { "identifier": "+919876543210", - "otp": "1234" + "code": "1234" } // Response (200 OK) @@ -109,26 +120,25 @@ ``` ### 2) Fare Estimate (`POST /customer/fare/estimate`) + +> **Corrected 11 Sep 2026** against `cxEstimateRequest` +> (`controllers/cxFareController.go`). The old shape used +> `pickup.latitude`/`longitude` (parsed as `lat`/`lng`), gave `pickup` a +> `stateCode`/`districtCode` it does not have, and described destinations as +> carrying a `packages` array of weights. The estimate is priced on +> `packageCount`; per-package weight is not known until the miler weighs it at +> the door. + ```json // Request { "pickup": { - "latitude": 13.0827, - "longitude": 80.2707, - "stateCode": "TN", - "districtCode": "CHN" + "lat": 13.0827, + "lng": 80.2707 }, "destinations": [ - { - "stateCode": "TN", - "districtCode": "CHN", - "packages": [{ "weightKg": 2.5 }] - }, - { - "stateCode": "KA", - "districtCode": "BLR", - "packages": [{ "weightKg": 1.0 }] - } + { "stateCode": "TN", "districtCode": "CHN", "packageCount": 2 }, + { "stateCode": "KA", "districtCode": "BLR", "packageCount": 1 } ] } @@ -149,6 +159,20 @@ ``` ### 3) Booking Creation (`POST /customer/bookings`) + +> **Corrected 11 Sep 2026.** The shape below previously did not match the +> parser (`cxCreateBookingRequest`, `controllers/cxBookingController.go`), and +> every mismatch failed **silently** through `BodyParser` — no error, just a +> zero value: +> +> | Was documented | Actually parsed | Effect of following the old doc | +> |---|---|---| +> | `pickup.latitude` / `longitude` | `pickup.lat` / `lng` | pickup coordinates `0` | +> | `pickup.contactName` / `contactPhone` | *not read at all* | dropped | +> | destination fields flat | nested under `details` | every recipient/address field dropped | +> | destination `latitude` / `longitude` | `details.pin.lat` / `lng` | drop coordinates `0` | +> | `estimate` absent from the doc | **is** read | the quote shown to the customer was not recorded | + ```json // Request { @@ -156,26 +180,27 @@ "pickup": { "title": "Home", "sub": "Flat 4B, Green Towers, Anna Nagar", - "latitude": 13.0827, - "longitude": 80.2707, - "contactName": "Alex Kumar", - "contactPhone": "+919876543210" + "lat": 13.0827, + "lng": 80.2707 }, "destinations": [ { - "recipientName": "Priya S", - "recipientPhone": "+919840123456", - "building": "12/A", - "street": "MG Road", - "landmark": "Near Metro", - "districtCode": "CHN", "stateCode": "TN", - "latitude": 13.0850, - "longitude": 80.2100, + "districtCode": "CHN", "packageCount": 1, - "codAmount": 450 + "details": { + "street": "MG Road", + "building": "12/A", + "landmark": "Near Metro", + "recipientName": "Priya S", + "recipientPhone": "+919840123456", + "instructions": "Ring the bell", + "pin": { "lat": 13.0850, "lng": 80.2100 }, + "codAmount": 450 + } } ], + "estimate": { "min": 240, "max": 310 }, "remarks": "Handle with care" } @@ -192,16 +217,18 @@ "pickup": { "title": "Home", "sub": "Flat 4B, Green Towers, Anna Nagar", - "latitude": 13.0827, - "longitude": 80.2707 + "lat": 13.0827, + "lng": 80.2707 }, "destinations": [ { "index": 0, + "stateCode": "TN", "stateName": "Tamil Nadu", + "districtCode": "CHN", "districtName": "Chennai", "packageCount": 1, - "codAmount": 450, + "details": { "recipientName": "Priya S", "codAmount": 450 }, "trackingId": null, "stage": null } diff --git a/docs/customer-app-handover-2026-09-15.md b/docs/customer-app-handover-2026-09-15.md new file mode 100644 index 0000000..ee3293f --- /dev/null +++ b/docs/customer-app-handover-2026-09-15.md @@ -0,0 +1,297 @@ +# Doormile backend → customer app · what changed + +**For:** the `doormile_cx` app team +**From:** Doormile backend +**Date:** 15 Sep 2026 +**Verified against:** production Postgres + Redis, and the code in this repo + +--- + +## Read this first + +Two things decide what you can do today: + +1. **Send the verification code as `code`, not `otp`.** That works against + production right now. The server-side `otp` alias described below is written + but **not deployed yet** — do not rely on it until we confirm it has shipped. +2. **Booking creation is currently blocked on our side**, for a reason that has + nothing to do with your app. See [Still blocked](#still-blocked-on-our-side). + Sign-in will work before booking does. + +Everything else here is context for why your existing integration was failing. + +--- + +## 1. Sign-in was broken by a field name, and it was our documentation's fault + +Your app posted the code as `otp`. The server only ever read `code`. So the +parsed value was always empty, the "no code supplied" branch always fired, and +**every** sign-in returned: + +``` +400 {"error":{"code":"invalid"},"message":"Enter the code we sent you"} +``` + +A correct code failed exactly the same way as a wrong one. No amount of SMS +gateway credit would have changed it. + +**This was our fault, not yours.** Two of our documents disagreed: + +| Document | Said | Correct? | +|---|---|---| +| `customer-app-api-crisp.md` | `otp` | ❌ wrong — you built against this | +| `openapi-customer.yaml` | `code` | ✅ right | + +`customer-app-api-crisp.md` has been corrected. + +### What to send + +```jsonc +POST /api/v1/customer/auth/otp/verify +{ + "identifier": "+919876543210", + "code": "1234" // ← `code`, always +} +``` + +**Response 200:** + +```jsonc +{ + "success": true, + "data": { + "accessToken": "eyJhbGciOi...", + "refreshToken": "d8f1e2a3...", // 64 hex chars + "expiresIn": 3600, // seconds + "customer": { "id": "cust_294", "name": "...", "phone": "+91...", "email": "" } + } +} +``` + +### About the `otp` alias + +We are adding server-side acceptance of `otp` as a **deprecated alias**, so +builds already on customers' phones start working without an app release. When +both keys are present, `code` wins. + +**It is not deployed yet.** Treat it as a safety net for old installs, not as a +reason to keep sending `otp`. Please migrate to `code`. + +--- + +## 2. Three request shapes in our docs did not match the server + +Every one of these failed **silently** — our parser ignores unknown keys, so a +wrong field name produced a zero value, not an error. No 400, no log, just a +booking with coordinates of `0` or a missing recipient. + +If you built any of these from `customer-app-api-crisp.md` before 11 Sep, they +need changing. + +### 2.1 `POST /customer/auth/otp/verify` + +| Was documented | Server actually reads | +|---|---| +| `otp` | `code` | + +### 2.2 `POST /customer/fare/estimate` + +| Was documented | Server actually reads | +|---|---| +| `pickup.latitude` / `pickup.longitude` | `pickup.lat` / `pickup.lng` | +| `pickup.stateCode` / `districtCode` | *not read — pickup has only lat/lng* | +| `destinations[].packages[].weightKg` | `destinations[].packageCount` | + +Per-package weight is not an input. The estimate is priced on package **count**; +real weight is not known until the miler weighs it at the door. + +**Correct request:** + +```jsonc +{ + "pickup": { "lat": 13.0827, "lng": 80.2707 }, + "destinations": [ + { "stateCode": "TN", "districtCode": "CHN", "packageCount": 2 }, + { "stateCode": "KA", "districtCode": "BLR", "packageCount": 1 } + ] +} +``` + +### 2.3 `POST /customer/bookings` — the one with the most wrong fields + +| Was documented | Server actually reads | If you send the old shape | +|---|---|---| +| `pickup.latitude` / `longitude` | `pickup.lat` / `lng` | pickup coordinates become **0** | +| `pickup.contactName` / `contactPhone` | *not read at all* | silently dropped | +| destination fields **flat** | nested under `details` | **every** recipient/address field dropped | +| destination `latitude` / `longitude` | `details.pin.lat` / `lng` | drop coordinates become **0** | +| `estimate` not documented | **is** read and stored | the quote shown to the customer is not recorded | + +**Correct request:** + +```jsonc +{ + "slotId": "slot_20260916_t2", + "pickup": { + "title": "Home", + "sub": "Flat 4B, Green Towers, Anna Nagar", + "lat": 13.0827, + "lng": 80.2707 + }, + "destinations": [ + { + "stateCode": "TN", + "districtCode": "CHN", + "packageCount": 2, + "details": { + "street": "MG Road", + "building": "12/A", + "landmark": "Near Metro", + "recipientName": "Priya S", + "recipientPhone": "+919840123456", + "instructions": "Ring the bell", + "pin": { "lat": 13.0850, "lng": 80.2100 }, + "codAmount": 450 + } + } + ], + "estimate": { "min": 240, "max": 310 }, + "remarks": "Handle with care" +} +``` + +**The booking *response* was also documented wrong** — it returns +`pickup.lat` / `lng`, not `latitude` / `longitude`. If you parse the response +for coordinates, check that too. + +--- + +## 3. `remarks` now actually saves + +The top-level `remarks` field you were already sending was being dropped — the +server's request struct had no field for it, so `BodyParser` discarded it and +the booking's note was empty for every customer-app booking. The admin console +displays and searches that column, so operators saw nothing. + +Fixed and **merged to main**. Keep sending it exactly as you are. + +--- + +## 4. Auth flow, confirmed working end to end + +We created a test customer through the live API and verified the whole +sequence. There is no separate "request OTP" step after signup — signup sends +the code itself. + +``` +POST /api/v1/customer/auth/signup { name, phone, email? } → 200 {sent, resendAfterSeconds} +POST /api/v1/customer/auth/otp/verify { identifier, code } → 200 {accessToken, refreshToken, ...} +``` + +For an existing account: + +``` +POST /api/v1/customer/auth/otp/request { identifier } → 200 +POST /api/v1/customer/auth/otp/verify { identifier, code } → 200 +``` + +Two behaviours worth coding for, both confirmed by testing: + +- **The code is single-use.** After a successful verify it is deleted. Logging + in again requires a fresh `otp/request` first — re-sending the same code + returns `401 invalid_otp`. +- **There is a 30-second resend cooldown.** Calling `otp/request` inside that + window does not issue a new code. Respect `resendAfterSeconds` from the + response rather than retrying blindly. + +### Refresh + +``` +POST /api/v1/customer/auth/refresh { refreshToken } +``` + +Refresh tokens **rotate** — the presented one is revoked and replaced. Replaying +an already-used refresh token **revokes every session for that customer**, so +never keep an old one around as a fallback. Store only the newest. + +Access tokens last 1 hour (`expiresIn: 3600`). + +--- + +## 5. Do not offer the Email tab yet + +``` +POST /api/v1/customer/auth/otp/request {"identifier":"someone@example.com"} +→ 500 {"error":{"code":"server_error"},"message":"Something went wrong"} +``` + +SMTP is not configured on production (`SMTP_HOST`, `SMTP_USER`, +`SMTP_PASSWORD` are all unset). Email sign-in fails every time. + +**Please hide or disable the Email option** until we confirm SMTP is live. +Offering a path that always fails is worse than not offering it. + +--- + +## Still blocked on our side + +**You will not be able to create a booking yet, no matter what you send.** + +Both serviceability tables are empty on production: + +``` +serviceablestates 0 rows +serviceabledistricts 0 rows +``` + +`CreateCxBooking` validates every destination against that catalogue, so with +zero rows every booking is rejected with: + +``` +400 {"message":"Every destination needs a serviceable state and district"} +``` + +And `GET /customer/serviceability/states` returns `200` with an **empty list**, +so your state picker has nothing to show in the first place. + +This is ours to fix — the seed data exists (`seed_customer_app.sql`, 5 states +including Tamil Nadu / Kerala / Karnataka / Telangana / Puducherry, 22 +districts) and simply has not been applied to production. We will confirm when +it has. + +**Until then:** sign-in and the catalogue endpoints are what you can integrate +against. Booking creation will return a 400 that is not your bug. + +--- + +## Summary — what you need to change + +| # | Change | Priority | +|---|---|---| +| 1 | Send the verification code as **`code`**, not `otp` | **Required** — nothing works without it | +| 2 | Fare estimate: `pickup.lat`/`lng`, `packageCount` (no `packages[].weightKg`) | Required | +| 3 | Booking: `pickup.lat`/`lng`, destination details nested under `details`, coords at `details.pin` | Required | +| 4 | Parse the booking response's `pickup.lat`/`lng` (not `latitude`/`longitude`) | Required | +| 5 | Send `estimate: {min, max}` on booking create | Recommended — it is the dispute record | +| 6 | Hide the Email sign-in tab | Recommended | +| 7 | Handle single-use codes + the 30s resend cooldown | Recommended | +| 8 | Store only the newest refresh token, never replay an old one | Recommended | + +--- + +## Status of the backend changes referenced here + +| Change | State | +|---|---| +| `remarks` saved on booking create | **Merged to main** | +| Doc corrections (`customer-app-api-crisp.md`) | **In review** | +| `otp` accepted as alias for `code` | **In review — not deployed** | +| Failed OTP send no longer burns the cooldown / rate limit | **In review** | +| Serviceability seed applied to production | **Not done** | +| SMTP configured for email OTP | **Not done** | + +"In review" means written and tested but not yet on `api.doormile.com`. Build +against `code` and the corrected shapes — those are correct regardless of +deployment order. We will confirm when the alias and the seed are live. + +Questions → the backend team. diff --git a/internal/assignment/ai_layer.go b/internal/assignment/ai_layer.go index 2b18200..3ca6aff 100644 --- a/internal/assignment/ai_layer.go +++ b/internal/assignment/ai_layer.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "strconv" + "strings" "time" "doormile/constants" @@ -263,9 +264,15 @@ func pickBestFromCandidates(candidates []*milerCandidate) *milerCandidate { // ─── AI layer HTTP call ────────────────────────────────────────────────────── func callDecisionEngine(booking *models.PickupBooking, candidates []aiCandidate) (aiDecisionResponse, error) { - baseURL := os.Getenv("AI_LAYER_BASE_URL") + // No hardcoded fallback. This used to default to the production AI layer, + // so a developer running the backend locally sent real booking and rider + // data to it without ever configuring anything. Unset now means "no AI + // layer": the caller already falls back to legacy scoring when this + // returns an error (see AI_LAYER_FALLBACK above), so degrading is the + // designed path rather than a new one. + baseURL := strings.TrimSpace(os.Getenv("AI_LAYER_BASE_URL")) if baseURL == "" { - baseURL = "https://routemate.workolik.com" + return aiDecisionResponse{}, fmt.Errorf("AI_LAYER_BASE_URL is not set") } now := time.Now() diff --git a/main.go b/main.go index 4ddc62e..31376b0 100644 --- a/main.go +++ b/main.go @@ -84,6 +84,15 @@ func main() { _ = godotenv.Load() cfg := config.Load() + // Refuse to start on configuration that would be unsafe rather than merely + // wrong. JWT_SECRET_KEY used to default to a literal in config.go, which + // meant a clone of this repository was enough to mint a valid token for any + // account on any deployment that had not overridden it. + if err := cfg.Validate(); err != nil { + utils.Error("invalid configuration — refusing to start", "error", err) + os.Exit(1) + } + utils.Info("Starting Doormile Backend...") // 2. Connect to Postgres, Redis & NATS diff --git a/scratch/debug_booking.go b/scratch/debug_booking.go index 7d41386..0f93678 100644 --- a/scratch/debug_booking.go +++ b/scratch/debug_booking.go @@ -6,12 +6,21 @@ import ( "database/sql" "fmt" "log" + "os" _ "github.com/lib/pq" ) func main() { - dsn := "host=31.97.228.132 user=admin password=Package@321# dbname=logistics port=5433 sslmode=disable" + // Built from the environment, not hardcoded. This line used to carry the + // production host and password in plaintext, in a file tracked by git — so + // the live database credential shipped with every clone. Export DB_HOST, + // DB_USER, DB_PASSWORD, DB_NAME and DB_PORT (or source .env) before running. + dsn := fmt.Sprintf( + "host=%s user=%s password=%s dbname=%s port=%s sslmode=disable", + os.Getenv("DB_HOST"), os.Getenv("DB_USER"), os.Getenv("DB_PASSWORD"), + os.Getenv("DB_NAME"), os.Getenv("DB_PORT"), + ) db, err := sql.Open("postgres", dsn) if err != nil { log.Fatalf("Open failed: %v", err) @@ -81,4 +90,4 @@ func main() { db.Exec("DELETE FROM pickupbookings WHERE bookingno = 'DM-BK-DEBUG-001'") fmt.Println("Cleaned up test row.") } -} \ No newline at end of file +} diff --git a/scratch/fix_booking_status_constraint.go b/scratch/fix_booking_status_constraint.go index a3f70f2..9692e29 100644 --- a/scratch/fix_booking_status_constraint.go +++ b/scratch/fix_booking_status_constraint.go @@ -6,12 +6,21 @@ import ( "database/sql" "fmt" "log" + "os" _ "github.com/lib/pq" ) func main() { - dsn := "host=31.97.228.132 user=admin password=Package@321# dbname=logistics port=5433 sslmode=disable" + // Built from the environment, not hardcoded. This line used to carry the + // production host and password in plaintext, in a file tracked by git — so + // the live database credential shipped with every clone. Export DB_HOST, + // DB_USER, DB_PASSWORD, DB_NAME and DB_PORT (or source .env) before running. + dsn := fmt.Sprintf( + "host=%s user=%s password=%s dbname=%s port=%s sslmode=disable", + os.Getenv("DB_HOST"), os.Getenv("DB_USER"), os.Getenv("DB_PASSWORD"), + os.Getenv("DB_NAME"), os.Getenv("DB_PORT"), + ) db, err := sql.Open("postgres", dsn) if err != nil { log.Fatalf("Open failed: %v", err) @@ -57,4 +66,4 @@ func main() { WHERE rel.relname = 'pickupbookings' AND con.contype = 'c' `).Scan(&def) fmt.Println("Current constraint:", def) -} \ No newline at end of file +} diff --git a/scratch/list_tables.go b/scratch/list_tables.go index 45f4181..e5c1de7 100644 --- a/scratch/list_tables.go +++ b/scratch/list_tables.go @@ -6,13 +6,22 @@ import ( "database/sql" "fmt" "log" + "os" _ "github.com/lib/pq" ) func main() { // DSN matches the one in connect.go and .env - dsn := "host=31.97.228.132 user=admin password=Package@321# dbname=logistics port=5433 sslmode=disable" + // Built from the environment, not hardcoded. This line used to carry the + // production host and password in plaintext, in a file tracked by git — so + // the live database credential shipped with every clone. Export DB_HOST, + // DB_USER, DB_PASSWORD, DB_NAME and DB_PORT (or source .env) before running. + dsn := fmt.Sprintf( + "host=%s user=%s password=%s dbname=%s port=%s sslmode=disable", + os.Getenv("DB_HOST"), os.Getenv("DB_USER"), os.Getenv("DB_PASSWORD"), + os.Getenv("DB_NAME"), os.Getenv("DB_PORT"), + ) db, err := sql.Open("postgres", dsn) if err != nil { log.Fatalf("Failed to open DB: %v", err) @@ -42,4 +51,4 @@ func main() { } fmt.Printf("Table: %s\n", name) } -} \ No newline at end of file +}