Secure settings page v2 by homeles · Pull Request #11 · homeles/MARS

homeles · 2025-08-06T18:57:54Z

No description provided.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

- Improved GitHub token handling in resolvers and routes - Added validation to prevent empty token requests - Fixed enterprise name loading in Reports page - Enhanced token validation script with better error messages - Updated documentation with authentication troubleshooting guide

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

This PR implements a comprehensive security update to protect the settings page and fix client-side credential exposure vulnerabilities. The changes move sensitive credentials from client-side code to secure server-side handling and add authentication protection to the settings page.

Key changes:

Removed all sensitive credentials from client-side code and moved authentication to server-side endpoints
Added username/password authentication protection for the Settings page
Implemented server-side token handling for GitHub API calls with fallback to environment variables

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
server/src/schema/resolvers.ts	Updated GraphQL resolvers to use server-side tokens and added validation
server/src/routes/migrationRoutes.ts	Added token fallback logic and new enterprises list endpoint
server/src/routes/configRoutes.ts	New endpoint to safely expose GitHub configuration status
server/src/routes/authRoutes.ts	New authentication endpoint with secure credential validation
server/src/index.ts	Updated GraphQL context to use environment tokens by default
server/scripts/check-github-config.sh	Added GitHub token validation script for container startup
server/Dockerfile	Modified to run validation script on startup
client/src/utils/AuthContext.tsx	New authentication context using server-side validation
client/src/pages/Settings.tsx	Updated to use server-side config and authentication
client/src/components/ProtectedRoute.tsx	New component to protect routes with authentication
client/src/components/LoginForm.tsx	New login form component
client/vite.config.ts	Removed sensitive environment variables from client build

Copilot · 2025-08-06T22:09:42Z

server/src/schema/resolvers.ts

-        'Authorization': token.startsWith('Bearer ') ? token : `Bearer ${token}`,
+        'Authorization': token && token.trim() !== '' ? 
+          (token.startsWith('Bearer ') ? token : `Bearer ${token}`) : 
+          `Bearer ${process.env.GITHUB_TOKEN || ''}`,


This complex ternary expression is duplicated in multiple places and difficult to read. Consider extracting it into a helper function like getAuthorizationHeader(token: string): string to improve maintainability and reduce duplication.

Suggested change

`Bearer ${process.env.GITHUB_TOKEN || ''}`,

'Authorization': getAuthorizationHeader(token),

Copilot · 2025-08-06T22:09:42Z

server/src/routes/authRoutes.ts

+    const padded2 = Buffer.concat([buf2, Buffer.alloc(maxLength - buf2.length)]);
+    return [padded1, padded2];
+  }
+


The padBuffers function is defined inside the route handler but could be extracted to module level or a utilities file since it's a pure function that could be reused elsewhere.

Suggested change

Copilot · 2025-08-06T22:09:43Z

client/src/pages/Settings.tsx

        variables: {
          enterpriseName: selectedEnterprise,
-          token: accessToken
+          token: '' // Token is stored server-side now and passed from there


Passing an empty string for token with a comment is not self-documenting. Consider using a constant like SERVER_SIDE_TOKEN or undefined to make the intent clearer.

Note: See the diff below for a potential fix:

@@ -371,7 +374,7 @@ await checkOrgAccessMutation({ variables: { enterpriseName: selectedEnterprise, - token: '' // Token is stored server-side now and passed from there + token: SERVER_SIDE_TOKEN // Token is stored server-side now and passed from there } });

Copilot · 2025-08-06T22:09:43Z

client/src/utils/AuthContext.tsx

+    // Check if we have a stored auth token
+    const storedAuth = sessionStorage.getItem('adminAuth');
+    return storedAuth === 'true';
+  });


Using sessionStorage for authentication state with a simple string check is insecure. Consider implementing proper JWT tokens or session-based authentication for better security.

Copilot · 2025-08-06T22:09:43Z

server/src/schema/resolvers.ts

        console.log(`[Sync] Starting migration sync for enterprise: ${enterpriseName}`);

+        // Use environment variable token if the passed token is empty (client-side security)
+        const accessToken = (token?.trim() === '' || !token) ? process.env.GITHUB_TOKEN : token;


This token fallback logic is repeated multiple times throughout the codebase. Consider extracting it into a utility function like getEffectiveToken(clientToken: string): string to reduce duplication and ensure consistent behavior.

Suggested change

const accessToken = (token?.trim() === '' || !token) ? process.env.GITHUB_TOKEN : token;

const accessToken = getEffectiveToken(token);

Copilot · 2025-08-06T22:09:43Z

server/scripts/check-github-config.sh

+  # Verify token format - simple check
+  if [[ ! "$GITHUB_TOKEN" =~ ^gh[pus]_[A-Za-z0-9_]+$ ]]; then
+    echo "⚠️  WARNING: GITHUB_TOKEN format looks unusual"
+    echo "   Token should start with 'ghp_', 'ghu_', or 'ghs_' for personal access tokens"


The regex pattern is hardcoded for GitHub token validation. Consider documenting the expected token formats or making this validation more flexible since GitHub may introduce new token formats in the future.

Suggested change

echo " Token should start with 'ghp_', 'ghu_', or 'ghs_' for personal access tokens"

# Update this regex as new token formats are introduced by GitHub

if [[ ! "$GITHUB_TOKEN" =~ ^gh(p|u|s|o|r|b)_[A-Za-z0-9_]+$ ]]; then

echo "⚠️ WARNING: GITHUB_TOKEN format looks unusual"

echo " Token should start with one of: 'ghp_', 'ghu_', 'ghs_', 'gho_', 'ghr_', or 'ghb_'"

echo " For latest token formats, see: https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/about-authentication-to-github"

homeles added 3 commits August 5, 2025 15:50

Adding user/password to access the settings view

288b4df

Update README to include Authentication details

d513b2f

Moving all sensitive credentials to server side

bdf855c

homeles requested a review from Copilot August 6, 2025 18:57

This comment was marked as outdated.

Sign in to view

homeles and others added 4 commits August 6, 2025 14:00

Update client/src/utils/AuthContext.tsx

c642064

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update server/src/routes/authRoutes.ts

b4dec30

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update docker-compose.yml

5312153

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

fe592b7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

homeles requested a review from Copilot August 6, 2025 19:01

This comment was marked as outdated.

Sign in to view

homeles requested a review from Copilot August 6, 2025 19:31

This comment was marked as outdated.

Sign in to view

homeles force-pushed the secure-settings-page-v2 branch from da9fd56 to d81b566 Compare August 6, 2025 20:59

homeles requested a review from Copilot August 6, 2025 20:59

This comment was marked as outdated.

Sign in to view

homeles force-pushed the secure-settings-page-v2 branch from d81b566 to f9b5d38 Compare August 6, 2025 21:26

homeles requested a review from Copilot August 6, 2025 21:28

This comment was marked as outdated.

Sign in to view

homeles force-pushed the secure-settings-page-v2 branch from f9b5d38 to 35b126c Compare August 6, 2025 21:34

Clean up docker-compose and README

af2369a

homeles requested a review from Copilot August 6, 2025 21:52

This comment was marked as outdated.

Sign in to view

homeles and others added 5 commits August 6, 2025 16:56

Update server/src/routes/authRoutes.ts

d6fc4d6

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update server/src/schema/resolvers.ts

f7f03d3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update server/src/schema/resolvers.ts

da57fb3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update client/src/pages/Dashboard.tsx

5e9fc17

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot recommendation fixes

6544e47

homeles requested a review from Copilot August 6, 2025 22:08

Copilot AI reviewed Aug 6, 2025

View reviewed changes

	`Bearer ${process.env.GITHUB_TOKEN \|\| ''}`,
	'Authorization': getAuthorizationHeader(token),

	const accessToken = (token?.trim() === '' \|\| !token) ? process.env.GITHUB_TOKEN : token;
	const accessToken = getEffectiveToken(token);

-    echo "   Token should start with 'ghp_', 'ghu_', or 'ghs_' for personal access tokens"
+  # Update this regex as new token formats are introduced by GitHub
+  if [[ ! "$GITHUB_TOKEN" =~ ^gh(p|u|s|o|r|b)_[A-Za-z0-9_]+$ ]]; then
+    echo "⚠️  WARNING: GITHUB_TOKEN format looks unusual"
+    echo "   Token should start with one of: 'ghp_', 'ghu_', 'ghs_', 'gho_', 'ghr_', or 'ghb_'"
+    echo "   For latest token formats, see: https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/about-authentication-to-github"

Conversation

homeles commented Aug 6, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants