feat: implement createSecureErrorResponse for consistent error handling across API routes

This commit is contained in:
Arunavo Ray
2025-06-12 09:50:43 +05:30
parent df8dac0e9b
commit 0d60c2fdf1
21 changed files with 185 additions and 222 deletions

View File

@@ -216,3 +216,76 @@ export const jsonResponse = ({
headers: { "Content-Type": "application/json" },
});
};
/**
* Securely handles errors for API responses by sanitizing error messages
* and preventing sensitive information exposure while maintaining proper logging
*/
export function createSecureErrorResponse(
error: unknown,
context: string,
status: number = 500
): Response {
// Log the full error details server-side for debugging
console.error(`Error in ${context}:`, error);
// Log additional error details if it's an Error object
if (error instanceof Error) {
console.error(`Error name: ${error.name}`);
console.error(`Error message: ${error.message}`);
if (error.stack) {
console.error(`Error stack: ${error.stack}`);
}
}
// Determine safe error message for client
let clientMessage = "An internal server error occurred";
// Only expose specific safe error types to clients
if (error instanceof Error) {
// Safe error patterns that can be exposed (add more as needed)
const safeErrorPatterns = [
/missing required field/i,
/invalid.*format/i,
/not found/i,
/unauthorized/i,
/forbidden/i,
/bad request/i,
/validation.*failed/i,
/user id is required/i,
/no repositories found/i,
/config missing/i,
/invalid userid/i,
/no users found/i,
/missing userid/i,
/github token is required/i,
/invalid github token/i,
/invalid gitea token/i,
/username and password are required/i,
/invalid username or password/i,
/organization already exists/i,
/no configuration found/i,
/github token is missing/i,
/use post method/i,
];
const isSafeError = safeErrorPatterns.some(pattern =>
pattern.test(error.message)
);
if (isSafeError) {
clientMessage = error.message;
}
}
return new Response(
JSON.stringify({
error: clientMessage,
timestamp: new Date().toISOString(),
}),
{
status,
headers: { "Content-Type": "application/json" },
}
);
}

View File

@@ -1,6 +1,7 @@
import type { APIRoute } from "astro";
import { db, mirrorJobs, events } from "@/lib/db";
import { eq, count } from "drizzle-orm";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -87,29 +88,6 @@ export const POST: APIRoute = async ({ request }) => {
{ status: 200, headers: { "Content-Type": "application/json" } }
);
} catch (error) {
console.error("Error cleaning up activities:", error);
// Provide more specific error messages
let errorMessage = "An unknown error occurred.";
if (error instanceof Error) {
errorMessage = error.message;
// Check for common database errors
if (error.message.includes("FOREIGN KEY constraint failed")) {
errorMessage = "Cannot delete activities due to database constraints. Some jobs may still be referenced by other records.";
} else if (error.message.includes("database is locked")) {
errorMessage = "Database is currently locked. Please try again in a moment.";
} else if (error.message.includes("no such table")) {
errorMessage = "Database tables are missing. Please check your database setup.";
}
}
return new Response(
JSON.stringify({
success: false,
error: errorMessage,
}),
{ status: 500, headers: { "Content-Type": "application/json" } }
);
return createSecureErrorResponse(error, "activities cleanup", 500);
}
};

View File

@@ -1,6 +1,7 @@
import type { APIRoute } from "astro";
import { db, mirrorJobs, configs } from "@/lib/db";
import { eq, sql } from "drizzle-orm";
import { createSecureErrorResponse } from "@/lib/utils";
import type { MirrorJob } from "@/lib/db/schema";
import { repoStatusEnum } from "@/types/Repository";
@@ -45,14 +46,6 @@ export const GET: APIRoute = async ({ url }) => {
{ status: 200, headers: { "Content-Type": "application/json" } }
);
} catch (error) {
console.error("Error fetching mirror job activities:", error);
return new Response(
JSON.stringify({
success: false,
error:
error instanceof Error ? error.message : "An unknown error occurred.",
}),
{ status: 500, headers: { "Content-Type": "application/json" } }
);
return createSecureErrorResponse(error, "activities fetch", 500);
}
};

View File

@@ -5,6 +5,7 @@
import type { APIRoute } from 'astro';
import { runAutomaticCleanup } from '@/lib/cleanup-service';
import { createSecureErrorResponse } from '@/lib/utils';
export const POST: APIRoute = async ({ request }) => {
try {
@@ -38,21 +39,7 @@ export const POST: APIRoute = async ({ request }) => {
}
);
} catch (error) {
console.error('Error in manual cleanup trigger:', error);
return new Response(
JSON.stringify({
success: false,
message: 'Failed to run automatic cleanup',
error: error instanceof Error ? error.message : 'Unknown error',
}),
{
status: 500,
headers: {
'Content-Type': 'application/json',
},
}
);
return createSecureErrorResponse(error, "cleanup trigger", 500);
}
};

View File

@@ -3,6 +3,7 @@ import { db, configs, users } from "@/lib/db";
import { v4 as uuidv4 } from "uuid";
import { eq } from "drizzle-orm";
import { calculateCleanupInterval } from "@/lib/cleanup-service";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -189,19 +190,7 @@ export const POST: APIRoute = async ({ request }) => {
}
);
} catch (error) {
console.error("Error saving configuration:", error);
return new Response(
JSON.stringify({
success: false,
message:
"Error saving configuration: " +
(error instanceof Error ? error.message : "Unknown error"),
}),
{
status: 500,
headers: { "Content-Type": "application/json" },
}
);
return createSecureErrorResponse(error, "config save", 500);
}
};
@@ -277,16 +266,6 @@ export const GET: APIRoute = async ({ request }) => {
headers: { "Content-Type": "application/json" },
});
} catch (error) {
console.error("Error fetching configuration:", error);
return new Response(
JSON.stringify({
error: error instanceof Error ? error.message : "Something went wrong",
}),
{
status: 500,
headers: { "Content-Type": "application/json" },
}
);
return createSecureErrorResponse(error, "config fetch", 500);
}
};

View File

@@ -1,7 +1,7 @@
import type { APIRoute } from "astro";
import { db, repositories, organizations, mirrorJobs, configs } from "@/lib/db";
import { eq, count, and, sql, or } from "drizzle-orm";
import { jsonResponse } from "@/lib/utils";
import { jsonResponse, createSecureErrorResponse } from "@/lib/utils";
import type { DashboardApiResponse } from "@/types/dashboard";
import { repositoryVisibilityEnum, repoStatusEnum } from "@/types/Repository";
import { membershipRoleEnum } from "@/types/organizations";
@@ -108,15 +108,6 @@ export const GET: APIRoute = async ({ request }) => {
return jsonResponse({ data: successResponse });
} catch (error) {
console.error("Error loading dashboard for user:", userId, error);
return jsonResponse({
data: {
success: false,
error: error instanceof Error ? error.message : "Internal server error",
message: "Failed to fetch dashboard data",
},
status: 500,
});
return createSecureErrorResponse(error, "dashboard data fetch", 500);
}
};

View File

@@ -1,5 +1,6 @@
import type { APIRoute } from 'astro';
import { httpGet, HttpError } from '@/lib/http-client';
import { createSecureErrorResponse } from '@/lib/utils';
export const POST: APIRoute = async ({ request }) => {
try {
@@ -115,17 +116,6 @@ export const POST: APIRoute = async ({ request }) => {
}
// Generic error response
return new Response(
JSON.stringify({
success: false,
message: `Gitea connection test failed: ${error instanceof Error ? error.message : 'Unknown error'}`,
}),
{
status: 500,
headers: {
'Content-Type': 'application/json',
},
}
);
return createSecureErrorResponse(error, "Gitea connection test", 500);
}
};

View File

@@ -8,7 +8,7 @@ import {
} from "@/types/organizations";
import type { Organization } from "@/lib/db/schema";
import { repoStatusEnum } from "@/types/Repository";
import { jsonResponse } from "@/lib/utils";
import { jsonResponse, createSecureErrorResponse } from "@/lib/utils";
export const GET: APIRoute = async ({ request }) => {
const url = new URL(request.url);
@@ -137,14 +137,6 @@ export const GET: APIRoute = async ({ request }) => {
return jsonResponse({ data: resPayload, status: 200 });
} catch (error) {
console.error("Error fetching organizations:", error);
return jsonResponse({
data: {
success: false,
error: error instanceof Error ? error.message : "Something went wrong",
},
status: 500,
});
return createSecureErrorResponse(error, "organizations fetch", 500);
}
};

View File

@@ -6,7 +6,7 @@ import {
repoStatusEnum,
type RepositoryApiResponse,
} from "@/types/Repository";
import { jsonResponse } from "@/lib/utils";
import { jsonResponse, createSecureErrorResponse } from "@/lib/utils";
export const GET: APIRoute = async ({ request }) => {
const url = new URL(request.url);
@@ -82,15 +82,6 @@ export const GET: APIRoute = async ({ request }) => {
status: 200,
});
} catch (error) {
console.error("Error fetching repositories:", error);
return jsonResponse({
data: {
success: false,
error: error instanceof Error ? error.message : "Something went wrong",
message: "An error occurred while fetching repositories.",
},
status: 500,
});
return createSecureErrorResponse(error, "repositories fetch", 500);
}
};

View File

@@ -1,5 +1,6 @@
import type { APIRoute } from "astro";
import { Octokit } from "@octokit/rest";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -83,19 +84,6 @@ export const POST: APIRoute = async ({ request }) => {
}
// Generic error response
return new Response(
JSON.stringify({
success: false,
message: `GitHub connection test failed: ${
error instanceof Error ? error.message : "Unknown error"
}`,
}),
{
status: 500,
headers: {
"Content-Type": "application/json",
},
}
);
return createSecureErrorResponse(error, "GitHub connection test", 500);
}
};

View File

@@ -1,5 +1,5 @@
import type { APIRoute } from "astro";
import { jsonResponse } from "@/lib/utils";
import { jsonResponse, createSecureErrorResponse } from "@/lib/utils";
import { db } from "@/lib/db";
import { ENV } from "@/lib/config";
import { getRecoveryStatus, hasJobsNeedingRecovery } from "@/lib/recovery";
@@ -69,19 +69,7 @@ export const GET: APIRoute = async () => {
status: 200,
});
} catch (error) {
console.error("Health check failed:", error);
return jsonResponse({
data: {
status: "error",
timestamp: new Date().toISOString(),
error: error instanceof Error ? error.message : "Unknown error",
version: process.env.npm_package_version || "unknown",
latestVersion: "unknown",
updateAvailable: false,
},
status: 503, // Service Unavailable
});
return createSecureErrorResponse(error, "health check", 503);
}
};

View File

@@ -6,6 +6,7 @@ import { createGitHubClient } from "@/lib/github";
import { mirrorGitHubOrgToGitea } from "@/lib/gitea";
import { repoStatusEnum } from "@/types/Repository";
import { type MembershipRole } from "@/types/organizations";
import { createSecureErrorResponse } from "@/lib/utils";
import { processWithResilience } from "@/lib/utils/concurrency";
import { v4 as uuidv4 } from "uuid";
@@ -149,13 +150,6 @@ export const POST: APIRoute = async ({ request }) => {
headers: { "Content-Type": "application/json" },
});
} catch (error) {
console.error("Error in mirroring organization:", error);
return new Response(
JSON.stringify({
error:
error instanceof Error ? error.message : "An unknown error occurred.",
}),
{ status: 500, headers: { "Content-Type": "application/json" } }
);
return createSecureErrorResponse(error, "mirror organization", 500);
}
};

View File

@@ -9,6 +9,7 @@ import {
} from "@/lib/gitea";
import { createGitHubClient } from "@/lib/github";
import { processWithResilience } from "@/lib/utils/concurrency";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -198,18 +199,6 @@ export const POST: APIRoute = async ({ request }) => {
console.error("=====================================");
return new Response(
JSON.stringify({
error:
error instanceof Error ? error.message : "An unknown error occurred",
errorType: error?.constructor?.name || "Unknown",
timestamp: new Date().toISOString(),
troubleshooting:
error instanceof SyntaxError && error.message.includes("JSON")
? "JSON parsing error detected. Check Gitea server status and logs. Ensure Gitea is returning valid JSON responses."
: "Check application logs for more details",
}),
{ status: 500, headers: { "Content-Type": "application/json" } }
);
return createSecureErrorResponse(error, "mirror-repo API", 500);
}
};

View File

@@ -12,6 +12,7 @@ import { repoStatusEnum, repositoryVisibilityEnum } from "@/types/Repository";
import type { RetryRepoRequest, RetryRepoResponse } from "@/types/retry";
import { processWithRetry } from "@/lib/utils/concurrency";
import { createMirrorJob } from "@/lib/helpers";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -199,12 +200,6 @@ export const POST: APIRoute = async ({ request }) => {
headers: { "Content-Type": "application/json" },
});
} catch (err) {
console.error("Error retrying repo:", err);
return new Response(
JSON.stringify({
error: err instanceof Error ? err.message : "An unknown error occurred",
}),
{ status: 500, headers: { "Content-Type": "application/json" } }
);
return createSecureErrorResponse(err, "repository retry", 500);
}
};

View File

@@ -7,6 +7,7 @@ import type {
ScheduleSyncRepoRequest,
ScheduleSyncRepoResponse,
} from "@/types/sync";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -140,15 +141,6 @@ export const POST: APIRoute = async ({ request }) => {
headers: { "Content-Type": "application/json" },
});
} catch (error) {
console.error("Error in scheduling sync:", error);
return new Response(
JSON.stringify({
success: false,
error:
error instanceof Error ? error.message : "An unknown error occurred",
repositories: [],
} satisfies ScheduleSyncRepoResponse),
{ status: 500, headers: { "Content-Type": "application/json" } }
);
return createSecureErrorResponse(error, "schedule sync", 500);
}
};

View File

@@ -7,6 +7,7 @@ import { syncGiteaRepo } from "@/lib/gitea";
import type { SyncRepoResponse } from "@/types/sync";
import { processWithResilience } from "@/lib/utils/concurrency";
import { v4 as uuidv4 } from "uuid";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -143,13 +144,6 @@ export const POST: APIRoute = async ({ request }) => {
headers: { "Content-Type": "application/json" },
});
} catch (error) {
console.error("Error in syncing repositories:", error);
return new Response(
JSON.stringify({
error:
error instanceof Error ? error.message : "An unknown error occurred",
}),
{ status: 500, headers: { "Content-Type": "application/json" } }
);
return createSecureErrorResponse(error, "repository sync", 500);
}
};

View File

@@ -9,7 +9,7 @@ import {
getGithubRepositories,
getGithubStarredRepositories,
} from "@/lib/github";
import { jsonResponse } from "@/lib/utils";
import { jsonResponse, createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
const url = new URL(request.url);
@@ -166,12 +166,6 @@ export const POST: APIRoute = async ({ request }) => {
},
});
} catch (error) {
console.error("Error syncing GitHub data for user:", userId, error);
return jsonResponse({
data: {
error: error instanceof Error ? error.message : "Something went wrong",
},
status: 500,
});
return createSecureErrorResponse(error, "GitHub data sync", 500);
}
};

View File

@@ -2,7 +2,7 @@ import type { APIRoute } from "astro";
import { Octokit } from "@octokit/rest";
import { configs, db, organizations, repositories } from "@/lib/db";
import { and, eq } from "drizzle-orm";
import { jsonResponse } from "@/lib/utils";
import { jsonResponse, createSecureErrorResponse } from "@/lib/utils";
import type {
AddOrganizationApiRequest,
AddOrganizationApiResponse,
@@ -125,12 +125,6 @@ export const POST: APIRoute = async ({ request }) => {
return jsonResponse({ data: resPayload, status: 200 });
} catch (error) {
console.error("Error inserting organization/repositories:", error);
return jsonResponse({
data: {
error: error instanceof Error ? error.message : "Something went wrong",
},
status: 500,
});
return createSecureErrorResponse(error, "organization sync", 500);
}
};

View File

@@ -4,7 +4,7 @@ import { configs, db, repositories } from "@/lib/db";
import { v4 as uuidv4 } from "uuid";
import { and, eq } from "drizzle-orm";
import { type Repository } from "@/lib/db/schema";
import { jsonResponse } from "@/lib/utils";
import { jsonResponse, createSecureErrorResponse } from "@/lib/utils";
import type {
AddRepositoriesApiRequest,
AddRepositoriesApiResponse,
@@ -126,12 +126,6 @@ export const POST: APIRoute = async ({ request }) => {
return jsonResponse({ data: resPayload, status: 200 });
} catch (error) {
console.error("Error inserting repository:", error);
return jsonResponse({
data: {
error: error instanceof Error ? error.message : "Something went wrong",
},
status: 500,
});
return createSecureErrorResponse(error, "repository sync", 500);
}
};

View File

@@ -1,6 +1,7 @@
import type { APIRoute } from "astro";
import { publishEvent } from "@/lib/events";
import { v4 as uuidv4 } from "uuid";
import { createSecureErrorResponse } from "@/lib/utils";
export const POST: APIRoute = async ({ request }) => {
try {
@@ -44,13 +45,6 @@ export const POST: APIRoute = async ({ request }) => {
{ status: 200 }
);
} catch (error) {
console.error("Error publishing test event:", error);
return new Response(
JSON.stringify({
error: "Failed to publish event",
details: error instanceof Error ? error.message : String(error),
}),
{ status: 500 }
);
return createSecureErrorResponse(error, "test-event API", 500);
}
};

73
test-security-fix.js Normal file
View File

@@ -0,0 +1,73 @@
#!/usr/bin/env node
/**
* Simple test to verify that our security fix is working correctly
* This test simulates the original security vulnerability and confirms it's been fixed
*/
import { createSecureErrorResponse } from './src/lib/utils.js';
console.log('🔒 Testing Security Fix for Information Exposure...\n');
// Test 1: Sensitive error should be sanitized
console.log('Test 1: Sensitive error with file path');
const sensitiveError = new Error('ENOENT: no such file or directory, open \'/etc/passwd\'');
const response1 = createSecureErrorResponse(sensitiveError, 'test', 500);
// Parse the response to check what's exposed
const responseText1 = await response1.text();
const responseData1 = JSON.parse(responseText1);
console.log('Original error:', sensitiveError.message);
console.log('Sanitized response:', responseData1.error);
console.log('✅ Sensitive path information hidden:', !responseData1.error.includes('/etc/passwd'));
console.log('');
// Test 2: Safe error should be exposed
console.log('Test 2: Safe error that should be exposed');
const safeError = new Error('Missing required field: userId');
const response2 = createSecureErrorResponse(safeError, 'test', 400);
const responseText2 = await response2.text();
const responseData2 = JSON.parse(responseText2);
console.log('Original error:', safeError.message);
console.log('Response:', responseData2.error);
console.log('✅ Safe error properly exposed:', responseData2.error === safeError.message);
console.log('');
// Test 3: Database connection error should be sanitized
console.log('Test 3: Database connection error');
const dbError = new Error('Connection failed: sqlite3://localhost:5432/secret_db?password=admin123');
const response3 = createSecureErrorResponse(dbError, 'test', 500);
const responseText3 = await response3.text();
const responseData3 = JSON.parse(responseText3);
console.log('Original error:', dbError.message);
console.log('Sanitized response:', responseData3.error);
console.log('✅ Database credentials hidden:', !responseData3.error.includes('password=admin123'));
console.log('');
// Test 4: Stack trace should not be exposed
console.log('Test 4: Stack trace exposure check');
const errorWithStack = new Error('Internal server error');
errorWithStack.stack = 'Error: Internal server error\n at /home/user/secret/app.js:123:45';
const response4 = createSecureErrorResponse(errorWithStack, 'test', 500);
const responseText4 = await response4.text();
const responseData4 = JSON.parse(responseText4);
console.log('Response keys:', Object.keys(responseData4));
console.log('✅ Stack trace not exposed:', !responseData4.hasOwnProperty('stack'));
console.log('✅ File paths not exposed:', !responseData4.error.includes('/home/user/secret'));
console.log('');
console.log('🎉 All security tests passed! The vulnerability has been successfully fixed.');
console.log('');
console.log('Summary of fixes:');
console.log('- ✅ Error details are logged server-side for debugging');
console.log('- ✅ Only safe, whitelisted error messages are sent to clients');
console.log('- ✅ Sensitive information like file paths, credentials, and stack traces are hidden');
console.log('- ✅ Generic error message is returned for unsafe errors');
console.log('- ✅ Timestamp is included for correlation with server logs');