Skip to content

Revert password changes #309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
justin-phxm merged 2 commits into from
Jul 11, 2026
Merged

Revert password changes #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e61d8c
Revert "fix: rename secret key for UI master password in ECS task def…
dastine0308 Jul 11, 2026
e264863
Revert "refactor: consolidate multiple passwords into a single master…
dastine0308 Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions packages/amplify/amplify/backend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,17 @@ TelemetryECSTaskDefinition.addContainer("TheContainer", {
PRIVATE_KEY: ecs.Secret.fromSecretsManager(
TelemetryBackendSecretsManagerPrivKey,
),
UI_MASTER_PASSWORD: ecs.Secret.fromSecretsManager(
DRIVER_NAME_UPDATE_PASSWORD: ecs.Secret.fromSecretsManager(
HeliosPasswords,
"UI_MASTER_PASSWORD",
"DRIVER_NAME_UPDATE_PASSWORD",
),
FINISH_LINE_UPDATE_PASSWORD: ecs.Secret.fromSecretsManager(
HeliosPasswords,
"FINISH_LINE_UPDATE_PASSWORD",
),
SNAPSHOT_PASSWORD: ecs.Secret.fromSecretsManager(
HeliosPasswords,
"SNAPSHOT_PASSWORD",
),
},
});
Expand Down
5 changes: 3 additions & 2 deletions packages/server/.env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ MQTT_PASSWORD=changeme
########################
# App secrets
########################
# Single master password shared across all frontend actions
UI_MASTER_PASSWORD=changeme
DRIVER_NAME_UPDATE_PASSWORD=changeme
FINISH_LINE_UPDATE_PASSWORD=changeme
SNAPSHOT_PASSWORD=changeme

########################
# Machine Learning
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/controllers/LapController/LapController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
convertToDecimalDegrees,
} from "@/utils/lapCalculations";
import { createLightweightApplicationLogger } from "@/utils/logger";
import { validateMasterPassword } from "@/utils/validatePassword";

import {
FINISH_LINE_LOCATION,
Expand Down Expand Up @@ -138,7 +137,8 @@ export class LapController implements LapControllerType {
): CoordUpdateResponse {
logger.info(JSON.stringify(newCoordInfo));
const { lat, long, password } = newCoordInfo;
if (!validateMasterPassword(password)) {
if (password !== process.env.FINISH_LINE_UPDATE_PASSWORD) {
logger.error("Invalid Password: " + password);
Comment on lines +140 to +141

@coderabbitai coderabbitai Bot Jul 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Password is logged in the error message and the env-var check is fail-open.

Two issues on this password check:

  1. CWE-532 — Sensitive data in logs: logger.error("Invalid Password: " + password) logs the raw password value. The other controllers (driver.controller.ts, snapshot.controller.ts) log a generic "Invalid password attempt" message without the value. This is inconsistent and leaks credentials to log files.

  2. Fail-open when FINISH_LINE_UPDATE_PASSWORD is unset: The raw comparison password !== process.env.FINISH_LINE_UPDATE_PASSWORD passes if both sides are undefined (env var not configured + missing password in payload). Unlike validateSnapshotPassword which throws when the env var is missing, this check silently allows access.

🔒 Proposed fix: remove password from log and guard against unset env var 
     const { lat, long, password } = newCoordInfo;
-    if (password !== process.env.FINISH_LINE_UPDATE_PASSWORD) {
-      logger.error("Invalid Password: " + password);
+    const finishLinePassword = process.env.FINISH_LINE_UPDATE_PASSWORD;
+    if (!finishLinePassword || password !== finishLinePassword) {
+      logger.error("Invalid password attempt for finish line update");
       return { error: "Invalid Password", invalidFields: ["password"] };
     }
📝 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.

Suggested change
if (password !== process.env.FINISH_LINE_UPDATE_PASSWORD) {
logger.error("Invalid Password: " + password);
if (password !== process.env.FINISH_LINE_UPDATE_PASSWORD) {
logger.error("Invalid Password: " + password);
Suggested change
if (password !== process.env.FINISH_LINE_UPDATE_PASSWORD) {
logger.error("Invalid Password: " + password);
const finishLinePassword = process.env.FINISH_LINE_UPDATE_PASSWORD;
if (!finishLinePassword || password !== finishLinePassword) {
logger.error("Invalid password attempt for finish line update");
return { error: "Invalid Password", invalidFields: ["password"] };
}
🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 140-140: Avoid logging sensitive data
Context: logger.error("Invalid Password: " + password)
Note: [CWE-532] Insertion of Sensitive Information into Log File.

(log-sensitive-data-typescript)

🤖 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 `@packages/server/src/controllers/LapController/LapController.ts` around lines
140 - 141, Update the password validation in the LapController check to reject
requests when FINISH_LINE_UPDATE_PASSWORD is unset, preventing missing
credentials from passing validation. Replace the logger.error message with a
generic invalid-password message that never includes the supplied password,
matching the behavior of the other controllers.

Source: Linters/SAST tools

return { error: "Invalid Password", invalidFields: ["password"] };
}
try {
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/controllers/routeControllers/driver.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { BackendController } from "../BackendController/BackendController";
import { type Request, type Response } from "express";

import { createApplicationLogger } from "@/utils/logger";
import { validateMasterPassword } from "@/utils/validatePassword";
import { validateDriverUpdatePassword } from "@/utils/validatePassword";

import {
type DriverHealthResponseDTO,
Expand Down Expand Up @@ -111,7 +111,7 @@ export const updateDriverInfo = async (
}

// Validate password
if (!validateMasterPassword(password)) {
if (!validateDriverUpdatePassword(password)) {
logger.warn(`Invalid password attempt for driver update - Rfid: ${Rfid}`);
return response.status(401).json({
error: "Invalid password",
Expand Down
4 changes: 2 additions & 2 deletions packages/server/src/controllers/routeControllers/snapshot.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { BackendController } from "../BackendController/BackendController";
import { type Request, type Response } from "express";

import { createApplicationLogger } from "@/utils/logger";
import { validateMasterPassword } from "@/utils/validatePassword";
import { validateSnapshotPassword } from "@/utils/validatePassword";

import type {
CreateSnapshotRequestDTO,
Expand Down Expand Up @@ -82,7 +82,7 @@ export const createSnapshot = async (
.json({ error: "snapshot_from and snapshot_to must be valid ISO date strings" });
}

if (!validateMasterPassword(password)) {
if (!validateSnapshotPassword(password)) {

@coderabbitai coderabbitai Bot Jul 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the asyncHandler implementation and error-handling middleware to verify
# whether raw error messages are sanitized before reaching the client.
rg -nC5 "asyncHandler" --type=ts packages/server/src/
rg -nC5 "error.*handler\|ErrorHandler\|err.*req.*res" --type=ts packages/server/src/ -g '!**/node_modules/**'

Repository: UCSolarCarTeam/Helios-Telemetry

Length of output: 5895

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== errorHandler implementation =="
cat -n packages/server/src/middleware/errorHandler.ts

echo
echo "== snapshot controller around validateSnapshotPassword =="
rg -nC6 "validateSnapshotPassword|createSnapshot|SNAPSHOT_PASSWORD" packages/server/src/controllers/routeControllers/snapshot.controller.ts

echo
echo "== server/app wiring for error middleware =="
rg -nC4 "app.use\\(|errorHandler|asyncHandler|express\\(" packages/server/src -g '!**/node_modules/**'

echo
echo "== startup validation for driver update password =="
rg -nC6 "validateDriverUpdatePassword|DRIVER_UPDATE_PASSWORD|throw new Error" packages/server/src

Repository: UCSolarCarTeam/Helios-Telemetry

Length of output: 23647

Avoid returning the missing-secret error to clients. validateSnapshotPassword() throws when SNAPSHOT_PASSWORD is unset, and errorHandler sends err.message in the JSON response, so the client can see SNAPSHOT_PASSWORD environment variable is not configured. Move this check to startup like validateDriverUpdatePassword() or map it to a generic 500.

🤖 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 `@packages/server/src/controllers/routeControllers/snapshot.controller.ts` at
line 85, Update the snapshot controller flow around validateSnapshotPassword so
an unset SNAPSHOT_PASSWORD is validated during application startup, following
the existing validateDriverUpdatePassword pattern, rather than allowing its
configuration error to reach clients. If startup validation is not applicable,
catch that failure and route a generic 500 response without exposing the
missing-secret message.

logger.warn("Invalid password attempt for snapshot creation");
return response.status(401).json({ error: "Invalid password" });
}
Expand Down
25 changes: 18 additions & 7 deletions packages/server/src/utils/validatePassword.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const validPassword = process.env.UI_MASTER_PASSWORD;
const validPassword = process.env.MQTT_PASSWORD;

if (!validPassword) {
throw new Error("UI_MASTER_PASSWORD environment variable is not configured");
throw new Error("MQTT_PASSWORD environment variable is not configured");
Comment on lines +1 to +4

@coderabbitai coderabbitai Bot Jul 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that DRIVER_NAME_UPDATE_PASSWORD is not referenced anywhere in the application code.
rg -n "DRIVER_NAME_UPDATE_PASSWORD" --type=ts -g '!**/*.env*' -g '!**/backend.ts'
# Also check if MQTT_PASSWORD is referenced beyond MQTT connection setup.
rg -nC3 "MQTT_PASSWORD" --type=ts -g '!**/*.env*' -g '!**/backend.ts'

Repository: UCSolarCarTeam/Helios-Telemetry

Length of output: 169

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== validatePassword.ts ==\n'
git ls-files 'packages/server/src/utils/validatePassword.ts' 'packages/server/src/**' | cat
echo
wc -l packages/server/src/utils/validatePassword.ts 2>/dev/null || true
echo
cat -n packages/server/src/utils/validatePassword.ts 2>/dev/null || true

printf '\n== References to DRIVER_NAME_UPDATE_PASSWORD ==\n'
rg -n -S "DRIVER_NAME_UPDATE_PASSWORD" .

printf '\n== References to MQTT_PASSWORD ==\n'
rg -n -C 3 -S "MQTT_PASSWORD" packages/server/src .

Repository: UCSolarCarTeam/Helios-Telemetry

Length of output: 9672

validateDriverUpdatePassword should read DRIVER_NAME_UPDATE_PASSWORD, not MQTT_PASSWORD. The new secret is wired into deployment, but this validator still gates driver updates on the MQTT broker password, so the two credentials remain coupled. Update the env var and error message here so driver updates use their own secret.

🤖 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 `@packages/server/src/utils/validatePassword.ts` around lines 1 - 4, Update
validateDriverUpdatePassword to read the DRIVER_NAME_UPDATE_PASSWORD environment
variable instead of MQTT_PASSWORD, and change the missing-configuration error
message to reference DRIVER_NAME_UPDATE_PASSWORD so driver update validation
uses its dedicated secret.

Source: Linters/SAST tools

}

/**
Expand All @@ -12,21 +12,32 @@ if (!validPassword) {
*/

/**
* Validates a password against the shared master password.
* Validates the driver update password against the configured environment variable.
*
* Used by sensitive operations. Backed by the UI_MASTER_PASSWORD environment
* variable.
* This password is required for sensitive operations like updating driver information
* in the database. The password is stored in the MQTT_PASSWORD environment variable.
*
* @param password - The password to validate
* @returns true if the password matches the configured password, false otherwise
*
* @example
* ```typescript
* if (!validateMasterPassword(req.body.password)) {
* if (!validateDriverUpdatePassword(req.body.password)) {
* return res.status(401).json({ error: "Invalid password" });
* }
* ```
*/
export function validateMasterPassword(password: string): boolean {
export function validateDriverUpdatePassword(password: string): boolean {
return password === validPassword;
}

/**
* Validates the snapshot management password against SNAPSHOT_PASSWORD env var.
*/
export function validateSnapshotPassword(password: string): boolean {
const snapshotPassword = process.env.SNAPSHOT_PASSWORD;
if (!snapshotPassword) {
throw new Error("SNAPSHOT_PASSWORD environment variable is not configured");
}
return password === snapshotPassword;
}
Loading