chore: improve code quality and documentation by AungMyoKyaw · Pull Request #3 · AungMyoKyaw/jsherlock

AungMyoKyaw · 2026-02-15T19:32:05Z

Summary

This PR improves code quality, documentation, and addresses security vulnerabilities in the jsherlock package.

Security Improvements

Updated axios from 0.18.0 to 1.13.5 to fix multiple critical vulnerabilities:
- CVE: Server-Side Request Forgery (SSRF)
- CVE: Cross-Site Request Forgery (CSRF)
- CVE: Inefficient Regular Expression Complexity (DoS)
- CVE: Credential Leakage via Absolute URL

Dependency Updates

Updated TypeScript from 3.2.4 to 5.7.3
Updated mocha from 5.2.0 to 11.1.2
Updated ts-node from 7.0.1 to 10.9.2
Updated @types/node from 10.12.18 to 22.13.5
Updated @types/mocha from 5.2.5 to 10.0.10
Added prettier 3.5.2 for code formatting

Code Quality Improvements

Added custom error classes (JsherLockError, SiteNotFoundError, NetworkError) for better error handling
Added input validation for username parameter
Improved TypeScript types and exports
Added stricter TypeScript configuration options
Added lint and format npm scripts

Documentation Improvements

Added SECURITY.md with security policy
Enhanced README with:
- Requirements section (Node.js >= 18)
- Format and lint commands
- Additional keywords for better discoverability

Configuration Improvements

Updated .gitignore with additional entries for modern tooling
Added engines field to specify Node.js >= 18 requirement
Added prepublishOnly script for safer npm publishes

Test plan

Run npm install to verify dependencies install correctly
Run npm run build to verify TypeScript compiles without errors
Run npm test to verify tests pass
Run npm run lint to verify code style is consistent

- Update dependencies to latest versions (axios 1.13.5, typescript 5.7.3, mocha 11.1.2) - Fix security vulnerabilities in axios (SSRF, CSRF, DoS) - Add SECURITY.md for security policy - Add lint and format scripts with prettier - Add engines field to specify Node.js >= 18 requirement - Improve TypeScript configuration with stricter type checking - Add missing TypeScript types and exports for better DX - Improve .gitignore with additional entries - Add custom error classes (JsherLockError, SiteNotFoundError, NetworkError) - Add input validation for username parameter - Add more keywords for better discoverability

Copilot

Pull request overview

This PR modernizes the jsherlock package by updating dependencies, improving error handling with custom error classes, enhancing TypeScript type safety, and expanding documentation. The changes address security vulnerabilities in axios and introduce stricter code quality standards.

Changes:

Updated axios from 0.18.0 to 1.13.5 to address multiple CVEs (SSRF, CSRF, DoS, credential leakage)
Added custom error classes (JsherLockError, SiteNotFoundError, NetworkError) for better error handling
Enhanced TypeScript configuration with stricter compiler options and updated from v3.2.4 to v5.7.3
Added input validation, improved type definitions, and enhanced documentation

Reviewed changes

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

Show a summary per file

File	Description
package.json	Updated axios to 1.13.5 and other dependencies; added lint/format scripts and Node.js version requirement
tsconfig.json	Updated target to ES2022, enabled stricter compiler options, added declaration maps and source maps
src/lib/index.ts	Added custom error classes, helper functions for error handling, timeout configuration, and improved type safety
src/index.ts	Added username validation, exported custom error classes, improved array initialization
src/data/index.ts	Added proper TypeScript interfaces, enhanced type safety, added error handling in siteInfo
readme.md	Comprehensive documentation updates with usage examples, API documentation, and error handling guide
SECURITY.md	Added security policy with vulnerability reporting process and best practices
.gitignore	Added modern tooling entries (IDE files, package manager lockfiles, OS-specific files)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-15T19:35:59Z

src/lib/index.ts

+        `Failed to check ${siteName}: ${error.message}`,
+        error
+      );
+    }


The error handling logic is inconsistent. When an AxiosError is caught, it throws a NetworkError, but when any other error is caught (line 96), it silently sets exist to false. This means if the SiteNotFoundError thrown at line 48 somehow reaches this catch block, or if there's any other error type, it will be silently suppressed. Consider either re-throwing non-AxiosError errors or logging them. Additionally, the NetworkError thrown at line 91-94 will prevent returning the Imessage object, which may not be the intended behavior based on the original code.

Suggested change

}

}

console.error(`Unexpected error while checking ${siteName}:`, error);

Copilot · 2026-02-15T19:36:00Z

src/index.ts


  constructor(userName: string) {
+    if (!userName || typeof userName !== 'string') {
+      throw new Error('Username must be a non-empty string');


The username validation throws a generic Error instead of using the custom JsherLockError class. For consistency with the error handling pattern established in this PR, consider throwing a JsherLockError or creating a specific ValidationError class that extends JsherLockError.

Suggested change

throw new Error('Username must be a non-empty string');

throw new JsherLockError('Username must be a non-empty string');

Copilot · 2026-02-15T19:36:00Z

src/lib/index.ts

+  const errorMsgRegex = new RegExp(errorMsg, 'g');
+  return !errorMsgRegex.test(body);


Creating a new RegExp on every call with user-provided errorMsg could be vulnerable to Regular Expression Denial of Service (ReDoS) if the errorMsg contains malicious regex patterns. Since errorMsg comes from the site configuration (data.json), this is only a concern if that data can be externally influenced. Consider adding validation for the regex pattern or using a try-catch around the RegExp construction. Additionally, the 'g' flag is unnecessary here since you're only testing for existence with .test(), not performing multiple matches.

Suggested change

const errorMsgRegex = new RegExp(errorMsg, 'g');

return !errorMsgRegex.test(body);

try {

const errorMsgRegex = new RegExp(errorMsg);

return !errorMsgRegex.test(body);

} catch {

// If errorMsg is not a valid regular expression, fall back to a safe substring check.

return !body.includes(errorMsg);

}

Copilot · 2026-02-15T19:36:00Z

src/lib/index.ts

-    ];
+    const statusCode = response.status;
+    const body = response.data;
+    const responseUrl = response.request.res.responseUrl as string;


Accessing "response.request.res.responseUrl" (line 69) is fragile and may cause runtime errors. This property path depends on the underlying HTTP adapter (Node.js http module) and may not exist in all environments or axios configurations. The response object might not have a request property, or request might not have a res property. Consider adding null checks or using optional chaining: "response.request?.res?.responseUrl" or falling back to response.config.url if responseUrl is undefined. This is especially important with the axios version upgrade.

Suggested change

const responseUrl = response.request.res.responseUrl as string;

const responseUrl =

(response.request?.res?.responseUrl as string | undefined) ??

(response.config?.url as string | undefined) ??

'';

Copilot · 2026-02-15T19:36:00Z

src/lib/index.ts

      case 'message':
-        const { errorMsg } = site;
-        const errorMsgRegex = new RegExp(errorMsg, 'g');
-        exist = typeof body == 'string' ? !body.match(errorMsgRegex) : true;
+        if (site.errorMsg) {
+          exist = handleMessageError(body, site.errorMsg);
+        }
        break;
      case 'response_url':
-        const { errorUrl } = site;
-        exist = responseUrl != errorUrl;
+        if (site.errorUrl) {
+          exist = handleResponseUrlError(responseUrl, site.errorUrl);
+        }


The conditional checks for site.errorMsg (line 76) and site.errorUrl (line 81) are good defensive programming, but if these properties are missing when the errorType expects them, exist will remain true (from line 51), which may not be the intended behavior. Consider setting exist to false or throwing an error when the required property for the errorType is missing. For example, if errorType is 'message' but errorMsg is undefined, the username existence check will incorrectly default to true.

Copilot · 2026-02-15T19:36:01Z

src/lib/index.ts

+export class JsherLockError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = 'JsherLockError';
+  }
+}
+
+export class SiteNotFoundError extends JsherLockError {
+  constructor(siteName: string) {
+    super(`Site "${siteName}" is not supported`);
+    this.name = 'SiteNotFoundError';
+  }
+}
+
+export class NetworkError extends JsherLockError {
+  constructor(message: string, public readonly cause?: Error) {
+    super(message);
+    this.name = 'NetworkError';
+  }
+}


The new custom error classes (JsherLockError, SiteNotFoundError, NetworkError) and the username validation added at line 18-19 in src/index.ts lack test coverage. Given that the repository has test coverage for the main functionality, these new error handling paths should also be tested to ensure they work correctly. Consider adding tests for: 1) Invalid username input (empty string, null, non-string), 2) SiteNotFoundError when checking an unsupported site, 3) NetworkError scenarios (mock axios failures).

Copilot · 2026-02-15T19:36:01Z

src/index.ts

+    if (!userName || typeof userName !== 'string') {
+      throw new Error('Username must be a non-empty string');
+    }
    this.userName = userName;


The username validation checks for empty strings but doesn't check for whitespace-only strings. A username like " " would pass the validation (!userName check will be false for non-empty strings). Consider using .trim() before validation or checking for whitespace-only strings: "if (!userName || typeof userName !== 'string' || !userName.trim())".

Suggested change

if (!userName || typeof userName !== 'string') {

throw new Error('Username must be a non-empty string');

}

this.userName = userName;

if (typeof userName !== 'string' || !userName.trim()) {

throw new Error('Username must be a non-empty string');

}

this.userName = userName.trim();

Copilot AI review requested due to automatic review settings February 15, 2026 19:32

Copilot started reviewing on behalf of AungMyoKyaw February 15, 2026 19:32 View session

Copilot AI reviewed Feb 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: improve code quality and documentation#3

chore: improve code quality and documentation#3
AungMyoKyaw wants to merge 1 commit intomasterfrom
chore/improve-code-quality-and-documentation

AungMyoKyaw commented Feb 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 15, 2026

Uh oh!

Copilot AI Feb 15, 2026

Uh oh!

Copilot AI Feb 15, 2026

Uh oh!

Copilot AI Feb 15, 2026

Uh oh!

Copilot AI Feb 15, 2026

Uh oh!

Copilot AI Feb 15, 2026

Uh oh!

Copilot AI Feb 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	}
	}
	console.error(`Unexpected error while checking ${siteName}:`, error);

	throw new Error('Username must be a non-empty string');
	throw new JsherLockError('Username must be a non-empty string');

		const errorMsgRegex = new RegExp(errorMsg, 'g');
		return !errorMsgRegex.test(body);

-  const errorMsgRegex = new RegExp(errorMsg, 'g');
-  return !errorMsgRegex.test(body);
+  try {
+    const errorMsgRegex = new RegExp(errorMsg);
+    return !errorMsgRegex.test(body);
+  } catch {
+    // If errorMsg is not a valid regular expression, fall back to a safe substring check.
+    return !body.includes(errorMsg);
+  }

Conversation

AungMyoKyaw commented Feb 15, 2026

Summary

Security Improvements

Dependency Updates

Code Quality Improvements

Documentation Improvements

Configuration Improvements

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants