[fix] Security update

[maro114510]

Merge Overview

This pull request introduces significant improvements to error handling, token management, and message security in the content.js script for the Feedly extension. The changes enhance reliability, user feedback, and robustness, particularly around API failures, authentication, and communication between extension components.

Error Handling Enhancements:

• Introduced a structured FeedlyError class, error codes (ErrorCode), and user-friendly messages (UserMessages) for consistent error reporting and debugging. API and DOM errors now use these for more informative feedback.
• Updated API-related functions (feedlyApiRequest, handleOpenViaAPI, handleOpenViaDOM, handleOpen) to throw and handle FeedlyError instances, providing clearer error propagation and user messages. [1] [2] [3] [4]

Token Management Improvements:

• Replaced simple token caching with a metadata-driven cache (tokenCache) that includes TTL expiration and localStorage integrity checks using a hash function, reducing stale token issues. [1] [2]

Message Security and Validation:

• Added sender validation to ensure messages are only processed from the extension itself, preventing spoofed requests.
• Added settings validation and sanitization to enforce safe defaults and prevent malformed input from causing errors or unexpected behavior. [1] [2]

General Reliability:

• Improved fallback logic in handleOpen to ensure user-facing error messages are clear and actionable when both API and DOM approaches fail.
• Enhanced logging for both API and DOM failures, aiding debugging and support. [1] [2]

[Copilot]

Pull request overview

This pull request introduces a comprehensive security and reliability update to the Feedly extension's content script. It establishes a structured error handling system, improves token caching with TTL and integrity checks, and adds message validation to prevent malicious requests.

Changes:

• Introduced structured error handling with FeedlyError class, error codes, and user-friendly messages for consistent error reporting across API and DOM operations
• Enhanced token caching with TTL expiration and localStorage integrity verification using hash-based change detection
• Added sender validation and settings sanitization to the message listener to prevent spoofed requests and malformed input

---

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

[Copilot]

The classifyHttpError function does not handle 4xx client errors other than 401, 403, and 429. For example, a 400 Bad Request or 404 Not Found would be classified as ErrorCode.UNKNOWN, which has the generic message "Something went wrong. Please try again." Consider adding a case for other 4xx errors (e.g., if (status >= 400 && status < 500) return ErrorCode.CLIENT_ERROR) with a more specific user message like "Invalid request. Please try again or contact support."

[Copilot]

The hash-based change detection in isTokenCacheValid could produce false positives (cache invalidation when data hasn't changed) due to hash collisions. While the djb2 algorithm is generally good, collisions are still possible. A more robust approach would be to store the actual token value in the cache and compare it directly with the parsed token from localStorage, or use a more collision-resistant comparison method. This is particularly important for security-sensitive token validation.

[Copilot]

The response.json() call at line 257 is not wrapped in error handling. If the response body is not valid JSON, this will throw an error that is not a FeedlyError, breaking the consistent error handling pattern. Consider wrapping this in a try-catch block and throwing a FeedlyError with an appropriate error code (e.g., ErrorCode.SERVER_ERROR) if JSON parsing fails.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

content.js:207

• There's a potential race condition in getAccessToken when multiple API requests are made concurrently. If two requests call getAccessToken simultaneously when the cache is invalid:

1. Both check isTokenCacheValid() and it returns false
2. Both call clearAccessTokenCache()
3. Both read from localStorage and parse the session
4. Both set tokenCache with potentially the same values (harmless) OR different timestamps

While this won't cause data corruption (JavaScript is single-threaded), it results in unnecessary duplicate localStorage reads and JSON parsing. Consider adding a simple lock mechanism or memoization pattern:

let tokenPromise = null;

async function getAccessToken() {
  if (isTokenCacheValid()) {
    return tokenCache.token;
  }
  
  if (tokenPromise) {
    return tokenPromise;
  }
  
  tokenPromise = (async () => {
    // ... existing logic ...
  })();
  
  try {
    return await tokenPromise;
  } finally {
    tokenPromise = null;
  }
}

This is a minor optimization issue but worth considering if the extension frequently makes concurrent API calls.

async function getAccessToken() {
  // Return cached token if still valid
  if (isTokenCacheValid()) {
    return tokenCache.token;
  }

  // Clear expired/invalid cache
  clearAccessTokenCache();

  try {
    const sessionData = localStorage.getItem("feedly.session");
    if (sessionData) {
      const session = JSON.parse(sessionData);
      if (session.feedlyToken) {
        tokenCache = {
          token: session.feedlyToken,
          cachedAt: Date.now(),
          sourceHash: simpleHash(sessionData)
        };
        return tokenCache.token;
      }
    }
  } catch (e) {
    console.warn("[Feedly Opener] Failed to get token from localStorage:", e);
  }

  return null;
}

---

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