From 01e3b3e1852a0aa8685ef5e386d0ea8e6d66301d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 14:48:44 +0000 Subject: [PATCH 1/6] Initial plan From 65247a00d965f6d2cb068717d9040360fd601733 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 14:52:17 +0000 Subject: [PATCH 2/6] fix: apply security and robustness improvements from code review Co-authored-by: damsleth <7300548+damsleth@users.noreply.github.com> --- DOCKER.md | 15 ++++++--- scripts/agent-setup.sh | 45 +++++++++++++++++++-------- scripts/agent-teardown.sh | 6 +++- scripts/docker.sh | 6 ++++ server/routes/auth.ts | 64 +++++++++++++++++++++++++++++++-------- 5 files changed, 105 insertions(+), 31 deletions(-) diff --git a/DOCKER.md b/DOCKER.md index 42589ebda0..7466d86635 100644 --- a/DOCKER.md +++ b/DOCKER.md @@ -424,24 +424,31 @@ Or capture the full session object server-side and base64 encode it: } ``` -**2. Set the environment variable:** +**2. Set the environment variables:** ```bash export TEST_SESSION_COOKIE="eyJwYXNzc...base64..." +export SESSION_INJECTION_SECRET="your-secret-token" ``` **3. Authenticate via the injection endpoint:** ```bash -curl http://localhost:9142/auth/inject-session +# Uses APP_URL from .agent-env or your configured port +curl -X POST "${APP_URL}/auth/inject-session" -H "X-Injection-Secret: ${SESSION_INJECTION_SECRET}" # Redirects to /timesheet with valid session ``` In Playwright tests: ```typescript -test('authenticated flow', async ({ page }) => { +test('authenticated flow', async ({ page, request }) => { // Inject session before testing protected routes - await page.goto(`${process.env.APP_URL}/auth/inject-session`) + await request.post(`${process.env.APP_URL}/auth/inject-session`, { + headers: { + 'X-Injection-Secret': process.env.SESSION_INJECTION_SECRET + } + }) // Now authenticated - test protected functionality + await page.goto(`${process.env.APP_URL}/timesheet`) await expect(page.locator('h1')).toContainText('Timesheet') }) ``` diff --git a/scripts/agent-setup.sh b/scripts/agent-setup.sh index c01f553e9c..ff7fa7355f 100755 --- a/scripts/agent-setup.sh +++ b/scripts/agent-setup.sh @@ -13,6 +13,10 @@ error() { echo -e "[agent-setup][ERROR] $*" >&2; } # --- Derive unique identifiers from worktree path --- WORKTREE_NAME=$(basename "$ROOT_DIR") +# Normalize worktree name for Docker Compose compatibility: +# - convert to lowercase +# - replace any non-alphanumeric characters with hyphens +NORMALIZED_WORKTREE_NAME="$(printf '%s' "$WORKTREE_NAME" | tr '[:upper:]' '[:lower:]' | sed -E 's/[^a-z0-9]+/-/g')" # Generate a stable port offset from worktree name (hash to 1-99 range, add to base 9100) PORT_OFFSET=$(echo -n "$WORKTREE_NAME" | cksum | cut -d' ' -f1) PORT_OFFSET=$((PORT_OFFSET % 99 + 1)) @@ -20,7 +24,7 @@ APP_PORT=$((9100 + PORT_OFFSET)) MONGO_PORT=$((27100 + PORT_OFFSET)) REDIS_PORT=$((6400 + PORT_OFFSET)) -export COMPOSE_PROJECT_NAME="did-${WORKTREE_NAME}" +export COMPOSE_PROJECT_NAME="did-${NORMALIZED_WORKTREE_NAME}" info "Worktree: $WORKTREE_NAME" info "Ports: app=$APP_PORT, mongo=$MONGO_PORT, redis=$REDIS_PORT" @@ -30,10 +34,23 @@ info "Project: $COMPOSE_PROJECT_NAME" PARENT_ENV="${ROOT_DIR}/../.env" if [[ -f "$PARENT_ENV" ]]; then info "Loading credentials from parent .env" - set -a - # shellcheck source=/dev/null - source "$PARENT_ENV" - set +a + while IFS='=' read -r key value; do + # Skip empty lines and comments + [[ -z "${key}" || "${key}" =~ ^[[:space:]]*# ]] && continue + case "${key}" in + MICROSOFT_CLIENT_ID|MICROSOFT_CLIENT_SECRET|TEST_SESSION_COOKIE|SESSION_INJECTION_SECRET) + # Only set from parent .env if not already present in environment + if [[ -z "${!key-}" && -n "${value}" ]]; then + # Strip surrounding double quotes if present + value="${value%\"}" + value="${value#\"}" + export "${key}=${value}" + fi + ;; + *) + ;; + esac + done < "$PARENT_ENV" fi # --- Required env vars (from parent .env, CI secrets, or host env) --- @@ -41,6 +58,7 @@ fi : "${MICROSOFT_CLIENT_SECRET:?Missing MICROSOFT_CLIENT_SECRET - set in environment or parent .env}" # Optional: for session injection TEST_SESSION_COOKIE="${TEST_SESSION_COOKIE:-}" +SESSION_INJECTION_SECRET="${SESSION_INJECTION_SECRET:-}" # --- Generate docker-compose.local.yml with credentials + port overrides --- info "Generating docker-compose.local.yml" @@ -50,11 +68,12 @@ services: ports: - "${APP_PORT}:9001" environment: - - MICROSOFT_CLIENT_ID=${MICROSOFT_CLIENT_ID} - - MICROSOFT_CLIENT_SECRET=${MICROSOFT_CLIENT_SECRET} - - MICROSOFT_REDIRECT_URI=http://localhost:${APP_PORT}/auth/azuread-openidconnect/callback - - ENABLE_SESSION_INJECTION=${TEST_SESSION_COOKIE:+true} - - TEST_SESSION_COOKIE=${TEST_SESSION_COOKIE} + - MICROSOFT_CLIENT_ID="${MICROSOFT_CLIENT_ID}" + - MICROSOFT_CLIENT_SECRET="${MICROSOFT_CLIENT_SECRET}" + - MICROSOFT_REDIRECT_URI="http://localhost:${APP_PORT}/auth/azuread-openidconnect/callback" + - ENABLE_SESSION_INJECTION="${TEST_SESSION_COOKIE:+true}" + - TEST_SESSION_COOKIE="${TEST_SESSION_COOKIE}" + - SESSION_INJECTION_SECRET="${SESSION_INJECTION_SECRET}" mongodb: ports: - "${MONGO_PORT}:27017" @@ -126,11 +145,11 @@ echo "MONGO_URL=mongodb://localhost:${MONGO_PORT}" echo "REDIS_URL=redis://localhost:${REDIS_PORT}" echo "COMPOSE_PROJECT=${COMPOSE_PROJECT_NAME}" echo "" -if [[ -n "$TEST_SESSION_COOKIE" ]]; then +if [[ -n "$TEST_SESSION_COOKIE" && -n "$SESSION_INJECTION_SECRET" ]]; then info "Session injection enabled. To authenticate:" - echo " curl http://localhost:${APP_PORT}/auth/inject-session" + echo " curl -X POST http://localhost:${APP_PORT}/auth/inject-session -H \"X-Injection-Secret: \$SESSION_INJECTION_SECRET\"" else - info "Session injection disabled (no TEST_SESSION_COOKIE provided)" + info "Session injection disabled (requires TEST_SESSION_COOKIE and SESSION_INJECTION_SECRET)" fi echo "" info "To tear down: ./scripts/agent-teardown.sh" diff --git a/scripts/agent-teardown.sh b/scripts/agent-teardown.sh index 702fd3e5d2..1504ec363f 100755 --- a/scripts/agent-teardown.sh +++ b/scripts/agent-teardown.sh @@ -11,7 +11,11 @@ info() { echo -e "[agent-teardown] $*"; } warn() { echo -e "[agent-teardown][WARN] $*"; } WORKTREE_NAME=$(basename "$ROOT_DIR") -export COMPOSE_PROJECT_NAME="did-${WORKTREE_NAME}" +# Normalize worktree name to match agent-setup: +# - convert to lowercase +# - replace any non-alphanumeric characters with hyphens +NORMALIZED_WORKTREE_NAME="$(printf '%s' "$WORKTREE_NAME" | tr '[:upper:]' '[:lower:]' | sed -E 's/[^a-z0-9]+/-/g')" +export COMPOSE_PROJECT_NAME="did-${NORMALIZED_WORKTREE_NAME}" # Parse arguments REMOVE_WORKTREE=0 diff --git a/scripts/docker.sh b/scripts/docker.sh index e14333a045..e1d77c75a9 100755 --- a/scripts/docker.sh +++ b/scripts/docker.sh @@ -39,6 +39,12 @@ check_docker() { error "Docker is not running. Please start Docker first." exit 1 fi + + if ! docker compose version &> /dev/null; then + error "Docker Compose plugin is not installed or enabled." + echo " Visit: https://docs.docker.com/compose/install/" + exit 1 + fi } # ───────────────────────────────────────────────────────────────────────────── diff --git a/server/routes/auth.ts b/server/routes/auth.ts index 952a606f98..dc710bab08 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -160,41 +160,79 @@ auth.get('/signout', signOutHandler) * Session injection for agent/e2e testing (development only) * * Allows agents and Playwright tests to authenticate without going through OAuth. - * Requires ENABLE_SESSION_INJECTION=true and TEST_SESSION_COOKIE to be set. + * Requires NODE_ENV=development, ENABLE_SESSION_INJECTION=true, and TEST_SESSION_COOKIE to be set. * The TEST_SESSION_COOKIE should be a base64-encoded JSON object containing * the user session data (copy from a real logged-in session). * * @example - * curl http://localhost:9142/auth/inject-session + * curl -X POST "$APP_URL/auth/inject-session" -H "X-Injection-Secret: $SESSION_INJECTION_SECRET" */ if ( + environment('NODE_ENV') === 'development' && environment('ENABLE_SESSION_INJECTION', false, { isSwitch: true }) && - environment('TEST_SESSION_COOKIE') + environment('TEST_SESSION_COOKIE') && + environment('SESSION_INJECTION_SECRET') ) { - auth.get('/inject-session', (request: Request, response: Response) => { + auth.post('/inject-session', (request: Request, response: Response) => { try { + // Verify secret token + const providedSecret = request.headers['x-injection-secret'] + const expectedSecret = environment('SESSION_INJECTION_SECRET') as string + if (providedSecret !== expectedSecret) { + debug('Session injection failed: invalid secret') + return response.status(403).json({ error: 'Invalid secret' }) + } + const sessionData = JSON.parse( Buffer.from( environment('TEST_SESSION_COOKIE') as string, 'base64' ).toString('utf8') ) - debug('Injecting session for user: %s', sessionData?.passport?.user?.mail) - Object.assign(request.session, sessionData) - request.session.save((error) => { - if (error) { - debug('Session injection failed: %s', error.message) - return response.status(500).json({ error: 'Session save failed' }) + + const passportData = sessionData?.passport + if (!passportData) { + debug('Session injection error: missing passport data in TEST_SESSION_COOKIE') + return response.status(400).json({ error: 'Invalid TEST_SESSION_COOKIE' }) + } + + debug('Injecting session for user: %s', passportData?.user?.mail) + + // Regenerate session before injection to prevent fixation + request.session.regenerate((regenerateError) => { + if (regenerateError) { + debug('Session regeneration failed: %s', regenerateError.message) + return response.status(500).json({ error: 'Session regeneration failed' }) } - const redirectUrl = sessionData?.passport?.user?.startPage || '/timesheet' - return response.redirect(redirectUrl) + + // Only copy allowlisted field (passport) onto new session + ;(request.session as any).passport = passportData + + request.session.save((error) => { + if (error) { + debug('Session injection failed: %s', error.message) + return response.status(500).json({ error: 'Session save failed' }) + } + + // Restrict redirectUrl to internal relative paths only + let redirectUrl = passportData?.user?.startPage || '/timesheet' + if ( + typeof redirectUrl !== 'string' || + !redirectUrl.startsWith('/') || + redirectUrl.startsWith('//') + ) { + redirectUrl = '/timesheet' + } + + return response.redirect(redirectUrl) + }) }) } catch (error) { debug('Session injection parse error: %s', (error as Error).message) return response.status(400).json({ error: 'Invalid TEST_SESSION_COOKIE' }) } }) - debug('Session injection route enabled at /auth/inject-session') + debug('Session injection route enabled at POST /auth/inject-session') } export default auth From 8fc6be949b0ed51d31ed285936aa4af4501efcf0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 14:54:02 +0000 Subject: [PATCH 3/6] fix: add timing-safe comparison and handle edge cases Co-authored-by: damsleth <7300548+damsleth@users.noreply.github.com> --- scripts/agent-setup.sh | 7 +++++-- server/routes/auth.ts | 20 ++++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/scripts/agent-setup.sh b/scripts/agent-setup.sh index ff7fa7355f..9a9e12b790 100755 --- a/scripts/agent-setup.sh +++ b/scripts/agent-setup.sh @@ -34,9 +34,12 @@ info "Project: $COMPOSE_PROJECT_NAME" PARENT_ENV="${ROOT_DIR}/../.env" if [[ -f "$PARENT_ENV" ]]; then info "Loading credentials from parent .env" - while IFS='=' read -r key value; do + while IFS= read -r line; do # Skip empty lines and comments - [[ -z "${key}" || "${key}" =~ ^[[:space:]]*# ]] && continue + [[ -z "${line}" || "${line}" =~ ^[[:space:]]*# ]] && continue + # Split only on first '=' to handle values containing '=' + key="${line%%=*}" + value="${line#*=}" case "${key}" in MICROSOFT_CLIENT_ID|MICROSOFT_CLIENT_SECRET|TEST_SESSION_COOKIE|SESSION_INJECTION_SECRET) # Only set from parent .env if not already present in environment diff --git a/server/routes/auth.ts b/server/routes/auth.ts index dc710bab08..bb9c425b9c 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -10,6 +10,7 @@ import { NextFunction, Request, Response, Router } from 'express' import passport from 'passport' import _ from 'underscore' import url from 'url' +import crypto from 'crypto' import { GENERIC_SIGNIN_FAILED, SigninError @@ -175,11 +176,22 @@ if ( ) { auth.post('/inject-session', (request: Request, response: Response) => { try { - // Verify secret token - const providedSecret = request.headers['x-injection-secret'] + // Verify secret token with timing-safe comparison + const providedSecretRaw = request.headers['x-injection-secret'] + const providedSecret = Array.isArray(providedSecretRaw) + ? providedSecretRaw[0] + : providedSecretRaw const expectedSecret = environment('SESSION_INJECTION_SECRET') as string - if (providedSecret !== expectedSecret) { - debug('Session injection failed: invalid secret') + + if ( + !providedSecret || + typeof providedSecret !== 'string' || + !crypto.timingSafeEqual( + Buffer.from(providedSecret), + Buffer.from(expectedSecret) + ) + ) { + debug('Session injection failed: invalid or missing secret') return response.status(403).json({ error: 'Invalid secret' }) } From d6ae7428747c48a13736ca022b5d04f211b2eb90 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 14:55:25 +0000 Subject: [PATCH 4/6] fix: add buffer length check and improve .env parsing robustness Co-authored-by: damsleth <7300548+damsleth@users.noreply.github.com> --- scripts/agent-setup.sh | 9 ++++++--- server/routes/auth.ts | 11 ++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/scripts/agent-setup.sh b/scripts/agent-setup.sh index 9a9e12b790..d6d9d6de69 100755 --- a/scripts/agent-setup.sh +++ b/scripts/agent-setup.sh @@ -37,6 +37,8 @@ if [[ -f "$PARENT_ENV" ]]; then while IFS= read -r line; do # Skip empty lines and comments [[ -z "${line}" || "${line}" =~ ^[[:space:]]*# ]] && continue + # Skip lines without '=' (malformed) + [[ ! "${line}" =~ = ]] && continue # Split only on first '=' to handle values containing '=' key="${line%%=*}" value="${line#*=}" @@ -44,9 +46,10 @@ if [[ -f "$PARENT_ENV" ]]; then MICROSOFT_CLIENT_ID|MICROSOFT_CLIENT_SECRET|TEST_SESSION_COOKIE|SESSION_INJECTION_SECRET) # Only set from parent .env if not already present in environment if [[ -z "${!key-}" && -n "${value}" ]]; then - # Strip surrounding double quotes if present - value="${value%\"}" - value="${value#\"}" + # Strip surrounding quotes (both single and double) + if [[ "${value}" =~ ^\"(.*)\"$ ]] || [[ "${value}" =~ ^\'(.*)\'$ ]]; then + value="${BASH_REMATCH[1]}" + fi export "${key}=${value}" fi ;; diff --git a/server/routes/auth.ts b/server/routes/auth.ts index bb9c425b9c..d184e25970 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -183,15 +183,24 @@ if ( : providedSecretRaw const expectedSecret = environment('SESSION_INJECTION_SECRET') as string + // Validate secret exists and lengths match before timing-safe comparison if ( !providedSecret || typeof providedSecret !== 'string' || + providedSecret.length !== expectedSecret.length + ) { + debug('Session injection failed: invalid or missing secret') + return response.status(403).json({ error: 'Invalid secret' }) + } + + // Timing-safe comparison to prevent timing attacks + if ( !crypto.timingSafeEqual( Buffer.from(providedSecret), Buffer.from(expectedSecret) ) ) { - debug('Session injection failed: invalid or missing secret') + debug('Session injection failed: secret mismatch') return response.status(403).json({ error: 'Invalid secret' }) } From 67f72edd95570a5a85a216f86609adf8d4856c4b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 14:56:34 +0000 Subject: [PATCH 5/6] refactor: use case-insensitive header lookup and DRY principle Co-authored-by: damsleth <7300548+damsleth@users.noreply.github.com> --- scripts/agent-setup.sh | 28 ++++++++++++++++++---------- server/routes/auth.ts | 6 ++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/scripts/agent-setup.sh b/scripts/agent-setup.sh index d6d9d6de69..8d81f075a4 100755 --- a/scripts/agent-setup.sh +++ b/scripts/agent-setup.sh @@ -11,6 +11,14 @@ cd "$ROOT_DIR" info() { echo -e "[agent-setup] $*"; } error() { echo -e "[agent-setup][ERROR] $*" >&2; } +# List of environment variables to load from parent .env +ALLOWED_ENV_VARS=( + "MICROSOFT_CLIENT_ID" + "MICROSOFT_CLIENT_SECRET" + "TEST_SESSION_COOKIE" + "SESSION_INJECTION_SECRET" +) + # --- Derive unique identifiers from worktree path --- WORKTREE_NAME=$(basename "$ROOT_DIR") # Normalize worktree name for Docker Compose compatibility: @@ -42,20 +50,20 @@ if [[ -f "$PARENT_ENV" ]]; then # Split only on first '=' to handle values containing '=' key="${line%%=*}" value="${line#*=}" - case "${key}" in - MICROSOFT_CLIENT_ID|MICROSOFT_CLIENT_SECRET|TEST_SESSION_COOKIE|SESSION_INJECTION_SECRET) + + # Check if key is in allowlist + for allowed_key in "${ALLOWED_ENV_VARS[@]}"; do + if [[ "${key}" == "${allowed_key}" ]]; then # Only set from parent .env if not already present in environment if [[ -z "${!key-}" && -n "${value}" ]]; then - # Strip surrounding quotes (both single and double) - if [[ "${value}" =~ ^\"(.*)\"$ ]] || [[ "${value}" =~ ^\'(.*)\'$ ]]; then - value="${BASH_REMATCH[1]}" - fi + # Strip surrounding quotes using simpler pattern matching + value="${value#[\'\"]}" + value="${value%[\'\"]}" export "${key}=${value}" fi - ;; - *) - ;; - esac + break + fi + done done < "$PARENT_ENV" fi diff --git a/server/routes/auth.ts b/server/routes/auth.ts index d184e25970..569f74fb92 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -177,10 +177,8 @@ if ( auth.post('/inject-session', (request: Request, response: Response) => { try { // Verify secret token with timing-safe comparison - const providedSecretRaw = request.headers['x-injection-secret'] - const providedSecret = Array.isArray(providedSecretRaw) - ? providedSecretRaw[0] - : providedSecretRaw + // Using request.get() for case-insensitive header lookup + const providedSecret = request.get('x-injection-secret') const expectedSecret = environment('SESSION_INJECTION_SECRET') as string // Validate secret exists and lengths match before timing-safe comparison From eea609fa3ff6ee90e1046de4b3486d786b049a8f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 14:57:56 +0000 Subject: [PATCH 6/6] fix: correct quote stripping pattern and use consistent session indexing Co-authored-by: damsleth <7300548+damsleth@users.noreply.github.com> --- scripts/agent-setup.sh | 8 +++++--- server/routes/auth.ts | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/agent-setup.sh b/scripts/agent-setup.sh index 8d81f075a4..2f86b9a4d3 100755 --- a/scripts/agent-setup.sh +++ b/scripts/agent-setup.sh @@ -56,9 +56,11 @@ if [[ -f "$PARENT_ENV" ]]; then if [[ "${key}" == "${allowed_key}" ]]; then # Only set from parent .env if not already present in environment if [[ -z "${!key-}" && -n "${value}" ]]; then - # Strip surrounding quotes using simpler pattern matching - value="${value#[\'\"]}" - value="${value%[\'\"]}" + # Strip surrounding quotes (both single and double) + value="${value#\"}" + value="${value%\"}" + value="${value#\'}" + value="${value%\'}" export "${key}=${value}" fi break diff --git a/server/routes/auth.ts b/server/routes/auth.ts index 569f74fb92..8b53ca63e4 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -225,7 +225,7 @@ if ( } // Only copy allowlisted field (passport) onto new session - ;(request.session as any).passport = passportData + request.session['passport'] = passportData request.session.save((error) => { if (error) {