[object Object]

← back to Charge And Explore

fix(smartcar): never send an invalid Connect mode (TK-10142)

55ae619dacbccbcbe272885962140ca01c1a239a · 2026-09-13 09:39:21 -0700 · Steve Abrams

Smartcar Connect accepts ONLY mode=live|simulated (docs default: live).
The provider defaulted to the literal "test", which is not a valid value,
so every authorize URL carried an invalid mode. Nobody caught it because
the existing connectUrl test never asserted mode at all — a positive-only
test that passed the whole time the value was wrong.

Normalize instead of forwarding: anything not exactly "simulated" resolves
to "live". Safe because connectUrl() is unreachable unless BOTH credentials
are set (server.ts:1261 returns {configured:false} first), so an unset
SMARTCAR_MODE can only ever apply on a deployment deliberately given keys.

Adds a NEGATIVE test proving the guard reddens on the injected old literal
(verified: 2 failures with the fault in, 0 after restore).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HXvqntqopFMBK4dJRm66tS

Files touched

Diff

commit 55ae619dacbccbcbe272885962140ca01c1a239a
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Sun Sep 13 09:39:21 2026 -0700

    fix(smartcar): never send an invalid Connect mode (TK-10142)
    
    Smartcar Connect accepts ONLY mode=live|simulated (docs default: live).
    The provider defaulted to the literal "test", which is not a valid value,
    so every authorize URL carried an invalid mode. Nobody caught it because
    the existing connectUrl test never asserted mode at all — a positive-only
    test that passed the whole time the value was wrong.
    
    Normalize instead of forwarding: anything not exactly "simulated" resolves
    to "live". Safe because connectUrl() is unreachable unless BOTH credentials
    are set (server.ts:1261 returns {configured:false} first), so an unset
    SMARTCAR_MODE can only ever apply on a deployment deliberately given keys.
    
    Adds a NEGATIVE test proving the guard reddens on the injected old literal
    (verified: 2 failures with the fault in, 0 after restore).
    
    Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01HXvqntqopFMBK4dJRm66tS
---
 backend/src/providers/telematics.ts |  9 +++++++-
 backend/test/telematics.test.ts     | 41 +++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/backend/src/providers/telematics.ts b/backend/src/providers/telematics.ts
index 256f599..5696363 100644
--- a/backend/src/providers/telematics.ts
+++ b/backend/src/providers/telematics.ts
@@ -51,7 +51,14 @@ export class SmartcarProvider implements TelematicsProvider {
   readonly id = "smartcar";
   private clientId = process.env.SMARTCAR_CLIENT_ID ?? "";
   private clientSecret = process.env.SMARTCAR_CLIENT_SECRET ?? "";
-  private mode = process.env.SMARTCAR_MODE ?? "test"; // "test" | "live" | "simulated"
+  // Smartcar Connect accepts ONLY "live" | "simulated" (docs default: "live").
+  // "test" was never a valid value, so the old `?? "test"` default put an
+  // invalid literal on every authorize URL. Normalize instead of forwarding:
+  // anything that is not exactly "simulated" resolves to "live". Safe because
+  // connectUrl() is unreachable unless BOTH credentials are set
+  // (server.ts:1261 returns {configured:false} first), so an unset
+  // SMARTCAR_MODE can only apply on a deployment deliberately given live keys.
+  private mode = process.env.SMARTCAR_MODE === "simulated" ? "simulated" : "live";
   private scope = "read_vehicle_info read_battery read_charge";
   private unit = "imperial";
 
diff --git a/backend/test/telematics.test.ts b/backend/test/telematics.test.ts
index 084c4f7..172469a 100644
--- a/backend/test/telematics.test.ts
+++ b/backend/test/telematics.test.ts
@@ -49,3 +49,44 @@ test("connectUrl builds a valid Smartcar authorize URL", () => {
   assert.ok(u.includes("scope=read_vehicle_info"));
   assert.ok(u.includes(encodeURIComponent("https://chargeandexplore.com/auth/smartcar/callback")));
 });
+
+// --- Smartcar Connect `mode` is an enum, not a free string (TK-10142) ---
+// Smartcar Connect accepts ONLY "live" | "simulated". The provider used to
+// default to "test", which is not a valid value, so every authorize URL
+// carried an invalid mode. The pre-existing "connectUrl builds a valid
+// Smartcar authorize URL" test passed the whole time it was broken — it never
+// asserted `mode`. These assert the enum directly, including the injected
+// fault, so the invalid-literal class cannot come back silently.
+
+const MODE_OF = (u: string) => new URL(u).searchParams.get("mode");
+const ALLOWED = new Set(["live", "simulated"]);
+
+function connectUrlWithMode(mode: string | undefined): string {
+  const prev = process.env.SMARTCAR_MODE;
+  if (mode === undefined) delete process.env.SMARTCAR_MODE;
+  else process.env.SMARTCAR_MODE = mode;
+  try {
+    return new SmartcarProvider().connectUrl("https://chargeandexplore.com/auth/smartcar/callback", "s");
+  } finally {
+    if (prev === undefined) delete process.env.SMARTCAR_MODE;
+    else process.env.SMARTCAR_MODE = prev;
+  }
+}
+
+test("connectUrl mode defaults to 'live' when SMARTCAR_MODE is unset", () => {
+  assert.equal(MODE_OF(connectUrlWithMode(undefined)), "live");
+});
+
+test("connectUrl honours an explicit 'simulated'", () => {
+  assert.equal(MODE_OF(connectUrlWithMode("simulated")), "simulated");
+});
+
+test("NEGATIVE: an invalid SMARTCAR_MODE never reaches the authorize URL", () => {
+  // "test" is the exact literal the old default shipped. Injecting it (and
+  // other junk) must NOT produce mode=test — it must normalize to "live".
+  for (const bad of ["test", "", "LIVE", "prod", "sandbox"]) {
+    const m = MODE_OF(connectUrlWithMode(bad));
+    assert.ok(ALLOWED.has(m ?? ""), `mode=${m} from SMARTCAR_MODE=${JSON.stringify(bad)} is not an allowed value`);
+    assert.notEqual(m, "test");
+  }
+});

← e74309a Make AdSense review pages crawlable and content-safe  ·  back to Charge And Explore  ·  (newest)