SCRAPE-23: Sanitize error messages in scraper_runs (#27)
Co-authored-by: Stephen Minakian <stephenminakian@gmail.com> Co-committed-by: Stephen Minakian <stephenminakian@gmail.com>
This commit is contained in:
@ -455,3 +455,222 @@ describe('runScrape', () => {
|
||||
expect(result.errors).toContain('No units found in HTML');
|
||||
});
|
||||
});
|
||||
|
||||
// ============================================================
|
||||
// Test: sanitizeError - Error message sanitization
|
||||
// ============================================================
|
||||
describe('sanitizeError', () => {
|
||||
let sanitizeError;
|
||||
|
||||
beforeAll(() => {
|
||||
({ sanitizeError } = require('../../services/scraperService'));
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------
|
||||
// File path sanitization
|
||||
// ----------------------------------------------------------
|
||||
describe('file path sanitization', () => {
|
||||
it('should remove Unix absolute file paths from error messages', () => {
|
||||
const error = new Error('ENOENT: no such file or directory, open /home/user/app/config.json');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toMatch(/\/home\/user/);
|
||||
expect(sanitized.message).toContain('ENOENT');
|
||||
});
|
||||
|
||||
it('should remove /var paths from error messages', () => {
|
||||
const error = new Error('Failed to read /var/app/current/data/secrets.yml');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toMatch(/\/var\/app/);
|
||||
expect(sanitized.message).toContain('Failed to read');
|
||||
});
|
||||
|
||||
it('should remove Windows-style file paths from error messages', () => {
|
||||
const error = new Error('Cannot find module C:\\Users\\admin\\project\\node_modules\\secret');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toMatch(/C:\\Users/);
|
||||
expect(sanitized.message).toContain('Cannot find module');
|
||||
});
|
||||
|
||||
it('should remove /tmp and /usr paths from error messages', () => {
|
||||
const error = new Error('Error loading /tmp/scraper-cache/data.bin and /usr/local/lib/node.so');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toMatch(/\/tmp\//);
|
||||
expect(sanitized.message).not.toMatch(/\/usr\//);
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------
|
||||
// MongoDB connection string sanitization
|
||||
// ----------------------------------------------------------
|
||||
describe('connection string sanitization', () => {
|
||||
it('should redact mongodb:// connection strings', () => {
|
||||
const error = new Error('Connection failed: mongodb://admin:s3cretP4ss@db.example.com:27017/apartments');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toContain('s3cretP4ss');
|
||||
expect(sanitized.message).not.toContain('admin:');
|
||||
expect(sanitized.message).toContain('Connection failed');
|
||||
expect(sanitized.message).toContain('[REDACTED_CONNECTION_STRING]');
|
||||
});
|
||||
|
||||
it('should redact mongodb+srv:// connection strings', () => {
|
||||
const error = new Error('Timeout connecting to mongodb+srv://user:password123@cluster0.abc.mongodb.net/mydb');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toContain('password123');
|
||||
expect(sanitized.message).not.toContain('user:');
|
||||
expect(sanitized.message).toContain('Timeout connecting to');
|
||||
expect(sanitized.message).toContain('[REDACTED_CONNECTION_STRING]');
|
||||
});
|
||||
|
||||
it('should redact connection string without credentials', () => {
|
||||
const error = new Error('Cannot connect to mongodb://localhost:27017/apartments');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toMatch(/mongodb:\/\/localhost/);
|
||||
expect(sanitized.message).toContain('[REDACTED_CONNECTION_STRING]');
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------
|
||||
// Credential / secret sanitization
|
||||
// ----------------------------------------------------------
|
||||
describe('credential sanitization', () => {
|
||||
it('should redact common environment variable patterns', () => {
|
||||
const error = new Error('Invalid API_KEY=sk-abc123xyz or SECRET_TOKEN=bearer-9876');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toContain('sk-abc123xyz');
|
||||
expect(sanitized.message).not.toContain('bearer-9876');
|
||||
});
|
||||
|
||||
it('should redact password patterns', () => {
|
||||
const error = new Error('Auth failed with password=MyS3cret!');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).not.toContain('MyS3cret!');
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------
|
||||
// Error type preservation
|
||||
// ----------------------------------------------------------
|
||||
describe('error type preservation', () => {
|
||||
it('should preserve the error type/name', () => {
|
||||
const error = new TypeError('Cannot read properties of undefined at /home/user/app/server.js:42');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.name).toBe('TypeError');
|
||||
});
|
||||
|
||||
it('should preserve custom error names', () => {
|
||||
const error = new Error('Timeout at /var/app/scraper.js:100');
|
||||
error.name = 'TimeoutError';
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.name).toBe('TimeoutError');
|
||||
});
|
||||
|
||||
it('should preserve error name for RangeError', () => {
|
||||
const error = new RangeError('Maximum call stack size exceeded');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.name).toBe('RangeError');
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------
|
||||
// General description preserved for debugging
|
||||
// ----------------------------------------------------------
|
||||
describe('general description preservation', () => {
|
||||
it('should preserve a useful general description', () => {
|
||||
const error = new Error('Network timeout after 30000ms');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).toContain('Network timeout after 30000ms');
|
||||
});
|
||||
|
||||
it('should preserve error description when no sensitive data present', () => {
|
||||
const error = new Error('Request failed with status code 500');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).toBe('Request failed with status code 500');
|
||||
});
|
||||
|
||||
it('should return a useful message even after heavy sanitization', () => {
|
||||
const error = new Error('ECONNREFUSED mongodb://root:pass@host:27017 at /home/user/node_modules/mongodb/lib/connection.js:123');
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.message).toContain('ECONNREFUSED');
|
||||
expect(sanitized.message.length).toBeGreaterThan(5);
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------
|
||||
// Stack trace sanitization
|
||||
// ----------------------------------------------------------
|
||||
describe('stack trace removal', () => {
|
||||
it('should remove file paths from stack traces', () => {
|
||||
const error = new Error('Something failed');
|
||||
error.stack = 'Error: Something failed\n at Object.<anonymous> (/home/user/app/services/scraperService.js:42:10)\n at Module._compile (/usr/lib/node_modules/node/internal/modules/cjs/loader.js:1078:30)';
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.stack).not.toMatch(/\/home\/user/);
|
||||
expect(sanitized.stack).not.toMatch(/\/usr\/lib/);
|
||||
});
|
||||
|
||||
it('should handle errors without stack trace', () => {
|
||||
const error = new Error('No stack');
|
||||
error.stack = undefined;
|
||||
const sanitized = sanitizeError(error);
|
||||
expect(sanitized.stack).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------
|
||||
// Integration: sanitization applied before recordScraperRun()
|
||||
// ----------------------------------------------------------
|
||||
describe('sanitization before recordScraperRun()', () => {
|
||||
it('should store sanitized error message in scraper_runs on failure', async () => {
|
||||
const html = createSampleHtml(['SANITIZE-A']);
|
||||
|
||||
// Create a db proxy that throws an error containing sensitive info
|
||||
const sensitiveError = new Error(
|
||||
'MongoServerError: connection to mongodb://admin:SuperSecret@db.prod.internal:27017/apartments failed at /home/deploy/app/node_modules/mongodb/lib/connection.js:370'
|
||||
);
|
||||
sensitiveError.name = 'MongoServerError';
|
||||
|
||||
const faultyDb = {
|
||||
collection: (name) => {
|
||||
const realCollection = db.collection(name);
|
||||
if (name === 'units_migration_test') {
|
||||
return new Proxy(realCollection, {
|
||||
get(target, prop) {
|
||||
if (prop === 'bulkWrite') {
|
||||
return async () => { throw sensitiveError; };
|
||||
}
|
||||
const value = target[prop];
|
||||
if (typeof value === 'function') {
|
||||
return value.bind(target);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
});
|
||||
}
|
||||
return realCollection;
|
||||
}
|
||||
};
|
||||
|
||||
const result = await runScrape(faultyDb, {
|
||||
jobId: 'test-sanitized-error',
|
||||
htmlContent: html
|
||||
});
|
||||
|
||||
expect(result.status).toBe('failed');
|
||||
|
||||
// Verify the error stored in result.errors is sanitized
|
||||
const errorMsg = result.errors[0];
|
||||
expect(errorMsg).not.toContain('SuperSecret');
|
||||
expect(errorMsg).not.toContain('admin:');
|
||||
expect(errorMsg).not.toContain('/home/deploy/');
|
||||
expect(errorMsg).toContain('MongoServerError');
|
||||
|
||||
// Verify the error stored in scraper_runs is sanitized
|
||||
const runRecord = await db.collection('scraper_runs').findOne({ jobId: 'test-sanitized-error' });
|
||||
expect(runRecord).toBeTruthy();
|
||||
expect(runRecord.status).toBe('failed');
|
||||
const storedError = runRecord.errors[0];
|
||||
expect(storedError).not.toContain('SuperSecret');
|
||||
expect(storedError).not.toContain('admin:');
|
||||
expect(storedError).not.toContain('/home/deploy/');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user