SVG trophy generation fails silently - broken GraphQL request body, unguarded null fields, token leak in logs

src/Helpers/Retry.ts

@@ -1,5 +1,6 @@
 import { ServiceError } from "../Types/index.ts";
 import { Logger } from "./Logger.ts";
+import { safeErrorMessage } from "./safeErrorMessage.ts";
 
 export type RetryCallbackProps = {
   attempt: number;
@@ -25,7 +26,7 @@ async function* createAsyncIterable<T>(
       }
 
       yield null;
-      Logger.error(e);
+      Logger.error(safeErrorMessage(e));
       await new Promise((resolve) => setTimeout(resolve, delay));
     }
   }

src/Helpers/__tests__/safeErrorMessage.test.ts

@@ -0,0 +1,27 @@
+import { assertEquals } from "../../../deps.ts";
+import { safeErrorMessage } from "../safeErrorMessage.ts";
+
+Deno.test("safeErrorMessage extracts only the message from an Error", () => {
+  const error = new Error("boom");
+  assertEquals(safeErrorMessage(error), "boom");
+});
+
+Deno.test("safeErrorMessage never surfaces extra properties attached to an Error", () => {
+  // soxa attaches the full request config (including the Authorization
+  // header) onto thrown errors. safeErrorMessage must only ever return
+  // .message, never those extra properties.
+  const error = new Error("request failed") as Error & { config: unknown };
+  error.config = {
+    headers: { Authorization: "bearer super-secret-token" },
+  };
+
+  const message = safeErrorMessage(error);
+
+  assertEquals(message, "request failed");
+  assertEquals(message.includes("super-secret-token"), false);
+});
+
+Deno.test("safeErrorMessage stringifies non-Error values", () => {
+  assertEquals(safeErrorMessage("plain string"), "plain string");
+  assertEquals(safeErrorMessage(null), "null");
+});

src/Helpers/safeErrorMessage.ts

@@ -0,0 +1,9 @@
+// Errors from soxa carry a .config with the Authorization header attached
+// (and a .toJSON() that serializes it back out), so only ever log the
+// message - never the raw error/cause/config, which could leak the token.
+export function safeErrorMessage(error: unknown): string {
+  if (error instanceof Error) {
+    return error.message;
+  }
+  return String(error);
+}

src/Services/GithubApiService.ts

@@ -19,6 +19,7 @@ import { CONSTANTS } from "../utils.ts";
 import { EServiceKindError, ServiceError } from "../Types/index.ts";
 import { Logger } from "../Helpers/Logger.ts";
 import { requestGithubData } from "./request.ts";
+import { safeErrorMessage } from "../Helpers/safeErrorMessage.ts";
 
 // Need to be here - Exporting from another file makes array of null
 export const TOKENS = [
@@ -71,8 +72,9 @@ export class GithubApiService extends GithubRepository {
         return result;
       }
       return UserInfo.fromCombined(result);
-    } catch {
+    } catch (error) {
       Logger.error(`Error fetching user info for username: ${username}`);
+      Logger.error(safeErrorMessage(error));
       return new ServiceError("Not found", EServiceKindError.NOT_FOUND);
     }
   }
@@ -98,11 +100,10 @@ export class GithubApiService extends GithubRepository {
         Logger.error(error.cause.message);
         return error.cause;
       }
-      if (error instanceof Error && error.cause) {
-        Logger.error(JSON.stringify(error.cause, null, 2));
-      } else {
-        Logger.error(error);
-      }
+      const cause = error instanceof Error && error.cause
+        ? error.cause
+        : error;
+      Logger.error(safeErrorMessage(cause));
       return new ServiceError("not found", EServiceKindError.NOT_FOUND);
     }
   }

src/Services/__tests__/request.test.ts

@@ -2,6 +2,35 @@ import { assertEquals, assertRejects, soxa, stub } from "../../../deps.ts";
 import { EServiceKindError, ServiceError } from "../../Types/index.ts";
 import { requestGithubData } from "../request.ts";
 
+Deno.test("requestGithubData sends query and variables as the POST body", async () => {
+  const post = stub(soxa, "post", () => {
+    return Promise.resolve({
+      data: { data: { user: { login: "test" } } },
+    });
+  });
+
+  try {
+    await requestGithubData(
+      "query { viewer { login } }",
+      { username: "test" },
+      "tok",
+    );
+
+    assertEquals(post.calls.length, 1);
+    const [, data] = post.calls[0].args;
+    // Regression check: soxa.post(url, data, config) uses the explicit
+    // `data` argument over `config.data` when merging, so the actual
+    // request body must be passed as the 2nd argument, not nested
+    // inside the 3rd (config) argument.
+    assertEquals(data, {
+      query: "query { viewer { login } }",
+      variables: { username: "test" },
+    });
+  } finally {
+    post.restore();
+  }
+});
+
 Deno.test("requestGithubData rejects partial GraphQL responses", async () => {
   const post = stub(soxa, "post", () => {
     return Promise.resolve({

src/Services/request.ts

@@ -11,8 +11,7 @@ export async function requestGithubData<T = unknown>(
   variables: { [key: string]: string },
   token = "",
 ) {
-  const response = await soxa.post("", {}, {
-    data: { query, variables },
+  const response = await soxa.post("", { query, variables }, {
     headers: {
       Authorization: `bearer ${token}`,
     },

src/__tests__/user_info.test.ts

@@ -39,3 +39,46 @@ Deno.test("UserInfo calculates total stargazers", () => {
 
   assertEquals(userInfo.totalStargazers, 8);
 });
+
+Deno.test("UserInfo defaults null/missing GraphQL sub-fields instead of throwing", () => {
+  // GitHub's GraphQL API can return null for fields like `organizations`
+  // when the token lacks the relevant scope, and null entries inside
+  // `repositories.nodes` for repos it can't fully resolve for the given
+  // token. UserInfo must degrade gracefully instead of throwing.
+  const userInfo = new UserInfo(
+    {
+      createdAt: "2025-01-01T00:00:00Z",
+      contributionsCollection: {
+        restrictedContributionsCount: 0,
+        totalCommitContributions: 0,
+        totalPullRequestReviewContributions: 0,
+      },
+      organizations: null as unknown as { totalCount: number },
+      followers: null as unknown as { totalCount: number },
+    },
+    {
+      openIssues: { totalCount: 0 },
+      closedIssues: { totalCount: 0 },
+    },
+    { pullRequests: { totalCount: 0 } },
+    {
+      repositories: {
+        totalCount: 1,
+        // deno-lint-ignore no-explicit-any
+        nodes: [
+          null as any,
+          {
+            languages: { nodes: [] },
+            stargazerCount: 5,
+            createdAt: "2025-01-02T00:00:00Z",
+          },
+        ],
+      },
+    },
+  );
+
+  assertEquals(userInfo.totalOrganizations, 0);
+  assertEquals(userInfo.totalFollowers, 0);
+  assertEquals(userInfo.totalStargazers, 5);
+  assertEquals(userInfo.totalRepositories, 1);
+});

src/user_info.ts

@@ -72,19 +72,23 @@ export class UserInfo {
     userPullRequest: GitHubUserPullRequest,
     userRepository: GitHubUserRepository,
   ) {
+    const repoNodes = (userRepository.repositories?.nodes ?? []).filter(
+      (node): node is Repository => node != undefined,
+    );
+
     const totalCommits =
-      userActivity.contributionsCollection.restrictedContributionsCount +
-      userActivity.contributionsCollection.totalCommitContributions;
-    const totalStargazers = userRepository.repositories.nodes.reduce(
+      (userActivity.contributionsCollection?.restrictedContributionsCount ?? 0) +
+      (userActivity.contributionsCollection?.totalCommitContributions ?? 0);
+    const totalStargazers = repoNodes.reduce(
       (prev: number, node: Repository) => {
-        return prev + node.stargazerCount;
+        return prev + (node.stargazerCount ?? 0);
       },
       0,
     );
 
     const languages = new Set<string>();
-    userRepository.repositories.nodes.forEach((node: Repository) => {
-      if (node.languages.nodes != undefined) {
+    repoNodes.forEach((node: Repository) => {
+      if (node.languages?.nodes != undefined) {
         node.languages.nodes.forEach((node: Language) => {
           if (node != undefined) {
             languages.add(node.name);
@@ -96,7 +100,7 @@ export class UserInfo {
     // Find the earliest repository creation date
     let earliestRepoDate = userActivity.createdAt; // start with the oldest possible
 
-    earliestRepoDate = userRepository.repositories.nodes.reduce(
+    earliestRepoDate = repoNodes.reduce(
       (earliest, node) => {
         return new Date(node.createdAt).getTime() < new Date(earliest).getTime()
           ? node.createdAt
@@ -118,15 +122,15 @@ export class UserInfo {
     const ogAccount = new Date(earliestRepoDate).getFullYear() <= 2008 ? 1 : 0;
 
     this.totalCommits = totalCommits;
-    this.totalFollowers = userActivity.followers.totalCount;
-    this.totalIssues = userIssue.openIssues.totalCount +
-      userIssue.closedIssues.totalCount;
-    this.totalOrganizations = userActivity.organizations.totalCount;
-    this.totalPullRequests = userPullRequest.pullRequests.totalCount;
+    this.totalFollowers = userActivity.followers?.totalCount ?? 0;
+    this.totalIssues = (userIssue.openIssues?.totalCount ?? 0) +
+      (userIssue.closedIssues?.totalCount ?? 0);
+    this.totalOrganizations = userActivity.organizations?.totalCount ?? 0;
+    this.totalPullRequests = userPullRequest.pullRequests?.totalCount ?? 0;
     this.totalReviews =
-      userActivity.contributionsCollection.totalPullRequestReviewContributions;
+      userActivity.contributionsCollection?.totalPullRequestReviewContributions ?? 0;
     this.totalStargazers = totalStargazers;
-    this.totalRepositories = userRepository.repositories.totalCount;
+    this.totalRepositories = userRepository.repositories?.totalCount ?? 0;
     this.languageCount = languages.size;
     this.durationYear = durationYear;
     this.durationDays = durationDays;