Skip to content

Conversation

@zdrapela
Copy link
Member

@zdrapela zdrapela commented Jan 29, 2026

Summary

This PR fixes a security issue where the Kubernetes bearer token was being leaked in CI logs when Kubernetes API errors occurred during E2E tests.

Problem

When the @kubernetes/client-node library throws an HttpError, the error object contains the full HTTP request/response, including the Authorization: Bearer <token> header. Logging the full error object (e.g., console.log(err)) exposed the token in CI logs.

Example of leaked data:

Authorization: Bearer XYZ...

Solution

Added a getKubeApiErrorMessage() helper function that safely extracts only non-sensitive information from Kubernetes API errors:

  • body.message, body.reason, body.code (standard K8s API error fields)
  • HTTP status code and message as fallback
  • Generic error message as last resort

Updated all error logging locations in kube-client.ts to use this safe extraction method instead of logging the full error object.

Changes

  • Added getKubeApiErrorMessage() utility function with proper TypeScript typing
  • Updated 12+ error handling locations to use safe error extraction
  • No changes to test logic or behavior, only error logging

Testing

  • The fix was verified by reviewing all console.log/console.error calls in kube-client.ts
  • All error paths now use the safe helper function
  • Error messages remain informative (e.g., "namespaces \"test-ns\" not found, reason: NotFound, code: 404")

https://issues.redhat.com/browse/RHDHBUGS-2563

@qodo-code-review
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Type Guard

isKubeApiError() currently returns true for any non-null object, which makes getKubeApiErrorMessage() treat many arbitrary errors as KubeApiError. Consider tightening the guard (e.g., check for known safe fields like body, response, statusCode, message) to reduce accidental assumptions and ensure consistent formatting.

function isKubeApiError(error: unknown): error is KubeApiError {
  return error !== null && typeof error === "object";
}
Falsy Checks

The helper uses truthy checks for numeric fields (e.g., code, statusCode). If any of these could be 0 or otherwise falsy, they would be omitted. Prefer nullish checks (e.g., != null) to avoid dropping valid numeric values.

if (err.body?.message) {
  const parts = [err.body.message];
  if (err.body.reason) parts.push(`reason: ${err.body.reason}`);
  if (err.body.code) parts.push(`code: ${err.body.code}`);
  return parts.join(", ");
}

// Fallback to statusCode and statusMessage from response
if (err.response?.statusCode) {
  return `HTTP ${err.response.statusCode}: ${err.response.statusMessage || "Unknown error"}`;
}

// Fallback to statusCode on error object
if (err.statusCode) {
  return `HTTP ${err.statusCode}`;
}
Error Context

Some paths wrap and rethrow errors with new Error(...), which can lose original stack/context. Consider using new Error(message, { cause: error }) (where supported) and/or compute the safe message once to avoid calling getKubeApiErrorMessage() multiple times and to preserve more debugging signal without leaking sensitive data.

} catch (error) {
  console.error(
    `Error updating ConfigMap: ${getKubeApiErrorMessage(error)}`,
  );
  throw new Error(
    `Failed to update ConfigMap: ${getKubeApiErrorMessage(error)}`,
  );
}
📄 References
  1. No matching references available

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 29, 2026

PR Type

(Describe updated until commit 8324024)

Bug fix, Enhancement


Description

  • Prevent Kubernetes bearer token leakage in error logs by extracting only safe error information

  • Added getKubeApiErrorMessage() helper function with type-safe error handling

  • Updated 12+ error logging locations to use safe error extraction instead of logging full error objects

  • Maintains informative error messages while protecting sensitive authentication data


File Walkthrough

Relevant files
Bug fix
kube-client.ts
Add safe Kubernetes API error logging utility                       

e2e-tests/playwright/utils/kube-client.ts

  • Added KubeApiError interface and isKubeApiError() type guard for safe
    error handling
  • Implemented getKubeApiErrorMessage() utility function that extracts
    only non-sensitive error information (message, reason, code, HTTP
    status) while excluding Authorization headers
  • Updated 12+ error logging calls throughout the file to use the safe
    helper function instead of logging full error objects
  • Replaced direct error object logging with formatted error messages
    that preserve debugging information without exposing tokens
+84/-17 

@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Type Guard

isKubeApiError() currently returns true for any non-null object, so getKubeApiErrorMessage() will treat arbitrary objects as KubeApiError. Consider tightening the guard (e.g., check for known keys and value types) to avoid unexpected stringification/undefined access patterns and to ensure the helper behaves predictably for non-Kubernetes errors.

function isKubeApiError(error: unknown): error is KubeApiError {
  return error !== null && typeof error === "object";
}

/**
 * Safely extracts error information from Kubernetes API errors without leaking sensitive data.
 * The @kubernetes/client-node HttpError contains the full HTTP request/response which includes
 * the Authorization header with the bearer token. This function extracts only safe information.
 */
function getKubeApiErrorMessage(error: unknown): string {
  if (isKubeApiError(error)) {
    const err = error;

    // Kubernetes API errors have a body with message, reason, and code
    if (err.body?.message) {
      const parts = [err.body.message];
      if (err.body.reason) parts.push(`reason: ${err.body.reason}`);
      if (err.body.code) parts.push(`code: ${err.body.code}`);
      return parts.join(", ");
    }

    // Fallback to statusCode and statusMessage from response
    if (err.response?.statusCode) {
      return `HTTP ${err.response.statusCode}: ${err.response.statusMessage || "Unknown error"}`;
    }

    // Fallback to statusCode on error object
    if (err.statusCode) {
      return `HTTP ${err.statusCode}`;
    }

    // Fallback to error message (safe as it doesn't contain request details)
    if (err.message) {
      return err.message;
    }
  }

  return "Unknown Kubernetes API error";
}
Secret Redaction

The fallback err.message is assumed safe, but in some HTTP/client libraries the message may include request details (potentially including headers). Consider adding a minimal redaction step (e.g., replace Authorization: Bearer ... patterns) or prefer structured fields over message to further reduce risk of token leakage in edge cases.

function getKubeApiErrorMessage(error: unknown): string {
  if (isKubeApiError(error)) {
    const err = error;

    // Kubernetes API errors have a body with message, reason, and code
    if (err.body?.message) {
      const parts = [err.body.message];
      if (err.body.reason) parts.push(`reason: ${err.body.reason}`);
      if (err.body.code) parts.push(`code: ${err.body.code}`);
      return parts.join(", ");
    }

    // Fallback to statusCode and statusMessage from response
    if (err.response?.statusCode) {
      return `HTTP ${err.response.statusCode}: ${err.response.statusMessage || "Unknown error"}`;
    }

    // Fallback to statusCode on error object
    if (err.statusCode) {
      return `HTTP ${err.statusCode}`;
    }

    // Fallback to error message (safe as it doesn't contain request details)
    if (err.message) {
      return err.message;
    }
  }

  return "Unknown Kubernetes API error";
}
Falsy Codes

Checks like if (err.body.code) and if (err.statusCode) drop values when they are 0 (or other falsy numbers). While uncommon for HTTP status codes, using != null (or typeof === "number") would be more correct and consistent for numeric fields.

// Kubernetes API errors have a body with message, reason, and code
if (err.body?.message) {
  const parts = [err.body.message];
  if (err.body.reason) parts.push(`reason: ${err.body.reason}`);
  if (err.body.code) parts.push(`code: ${err.body.code}`);
  return parts.join(", ");
}

// Fallback to statusCode and statusMessage from response
if (err.response?.statusCode) {
  return `HTTP ${err.response.statusCode}: ${err.response.statusMessage || "Unknown error"}`;
}

// Fallback to statusCode on error object
if (err.statusCode) {
  return `HTTP ${err.statusCode}`;
}
📄 References
  1. No matching references available

@sonarqubecloud
Copy link

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Sanitize errors before re-throwing them

To prevent potential token leaks in upstream error handlers, wrap the original
error in a new, sanitized error before re-throwing it from catch blocks, instead
of just sanitizing for logging.

Examples:

e2e-tests/playwright/utils/kube-client.ts [173-177]
      console.error(
        `Error finding app config ConfigMap: ${getKubeApiErrorMessage(error)}`,
      );
      throw error;
    }
e2e-tests/playwright/utils/kube-client.ts [809-813]
      console.error(
        `Error fetching services with label ${labelSelector}: ${getKubeApiErrorMessage(error)}`,
      );
      throw error;
    }

Solution Walkthrough:

Before:

class KubeClient {
  async findAppConfigMap(namespace: string): Promise<string> {
    try {
      // ... logic to find config map
    } catch (error) {
      console.error(
        `Error finding app config ConfigMap: ${getKubeApiErrorMessage(error)}`,
      );
      throw error; // The original, potentially sensitive error is re-thrown
    }
  }
}

After:

class KubeClient {
  async findAppConfigMap(namespace: string): Promise<string> {
    try {
      // ... logic to find config map
    } catch (error) {
      const safeMessage = getKubeApiErrorMessage(error);
      console.error(`Error finding app config ConfigMap: ${safeMessage}`);
      // Throw a new, sanitized error to prevent leaks
      throw new Error(safeMessage);
    }
  }
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical flaw where unsanitized errors are re-thrown, which undermines the PR's security goal by potentially leaking tokens in upstream error handlers.

High
General
Refine type guard for better accuracy

Refine the isKubeApiError type guard to be more specific by checking for
properties characteristic of Kubernetes API errors, such as body or response.

e2e-tests/playwright/utils/kube-client.ts [19-21]

 function isKubeApiError(error: unknown): error is KubeApiError {
-  return error !== null && typeof error === "object";
+  return (
+    error !== null &&
+    typeof error === "object" &&
+    ("body" in error || "response" in error || "statusCode" in error)
+  );
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the isKubeApiError type guard is too broad and proposes making it more specific, which improves the robustness of the new error handling logic.

Low
Handle non-object errors to prevent message loss

Modify getKubeApiErrorMessage to handle string-based errors by returning the
string directly, preventing the loss of the original error message.

e2e-tests/playwright/utils/kube-client.ts [28-57]

 function getKubeApiErrorMessage(error: unknown): string {
   if (isKubeApiError(error)) {
     const err = error;
 
     // Kubernetes API errors have a body with message, reason, and code
     if (err.body?.message) {
       const parts = [err.body.message];
       if (err.body.reason) parts.push(`reason: ${err.body.reason}`);
       if (err.body.code) parts.push(`code: ${err.body.code}`);
       return parts.join(", ");
     }
 
     // Fallback to statusCode and statusMessage from response
     if (err.response?.statusCode) {
       return `HTTP ${err.response.statusCode}: ${err.response.statusMessage || "Unknown error"}`;
     }
 
     // Fallback to statusCode on error object
     if (err.statusCode) {
       return `HTTP ${err.statusCode}`;
     }
 
     // Fallback to error message (safe as it doesn't contain request details)
     if (err.message) {
       return err.message;
     }
   }
 
+  if (typeof error === "string") {
+    return error;
+  }
+
   return "Unknown Kubernetes API error";
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a scenario where a string error message would be lost and provides a simple fix, improving the robustness of the new utility function.

Low
Preserve original error cause

When re-throwing an error in updateConfigMapTitle, include the original error in
the cause property of the new Error to preserve its stack trace.

e2e-tests/playwright/utils/kube-client.ts [390-392]

 throw new Error(
   `Failed to update ConfigMap: ${getKubeApiErrorMessage(error)}`,
+  { cause: error }
 );
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly proposes using the cause property to chain errors, which improves debuggability by preserving the original error's stack trace.

Low
  • More

@rhdh-qodo-merge
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Do not rethrow original error

In the KubeClient constructor, throw a new Error with the sanitized message from
getKubeApiErrorMessage instead of rethrowing the original error object to
prevent leaking sensitive data.

e2e-tests/playwright/utils/kube-client.ts [94-97]

 console.log(
   `Error initializing KubeClient: ${getKubeApiErrorMessage(e)}`,
 );
-throw e;
+throw new Error(`Error initializing KubeClient: ${getKubeApiErrorMessage(e)}`);
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a critical suggestion that aligns with the PR's main goal of preventing sensitive data leaks by ensuring the original, potentially unsafe, error object is not re-thrown.

Medium
Wrap error in sanitized Error

In updateConfigMap, throw a new Error with the sanitized message from
getKubeApiErrorMessage instead of rethrowing the original error to avoid leaking
sensitive HTTP details.

e2e-tests/playwright/utils/kube-client.ts [278-279]

 console.log(`Error updating configmap: ${getKubeApiErrorMessage(e)}`);
-throw e;
+throw new Error(getKubeApiErrorMessage(e));
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a critical suggestion that aligns with the PR's main goal of preventing sensitive data leaks by ensuring the original, potentially unsafe, error object is not re-thrown.

Medium
General
Ensure safe access to error properties

In waitForDeploymentReady, check the sanitized error message from
getKubeApiErrorMessage for specific content instead of directly accessing
error.message to ensure safer and more consistent error handling.

e2e-tests/playwright/utils/kube-client.ts [676-682]

-console.error(
-  `Error checking deployment status: ${getKubeApiErrorMessage(error)}`,
-);
+const errorMessage = getKubeApiErrorMessage(error);
+console.error(`Error checking deployment status: ${errorMessage}`);
 // If we threw an error about pod failure, re-throw it
-if (error.message?.includes("failed to start")) {
+if (errorMessage.includes("failed to start")) {
   throw error;
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that checking the result of getKubeApiErrorMessage is more robust and consistent with the PR's goal than accessing error.message directly.

Medium
Strengthen Kube error guard

Refine the isKubeApiError type guard to check for specific Kubernetes error
properties like body, response, or statusCode, making it more accurate and
preventing misclassification of other error objects.

e2e-tests/playwright/utils/kube-client.ts [19-21]

 function isKubeApiError(error: unknown): error is KubeApiError {
-  return error !== null && typeof error === "object";
+  return (
+    error !== null &&
+    typeof error === "object" &&
+    ("body" in error || "response" in error || "statusCode" in error)
+  );
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the isKubeApiError type guard is too broad and provides a more robust implementation that improves the safety of the new error handling logic.

Medium
  • More

@github-actions
Copy link
Contributor

@albarbaro
Copy link
Member

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Jan 29, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: albarbaro

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 239294c into redhat-developer:main Jan 29, 2026
28 checks passed
@openshift-ci
Copy link

openshift-ci bot commented Jan 29, 2026

@zdrapela: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ocp-helm 8324024 link unknown /test e2e-ocp-helm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@zdrapela
Copy link
Member Author

/cherry-pick release 1.9

@openshift-cherrypick-robot
Copy link
Contributor

@zdrapela: cannot checkout release: error checking out "release": exit status 1 error: pathspec 'release' did not match any file(s) known to git

Details

In response to this:

/cherry-pick release 1.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@zdrapela
Copy link
Member Author

/cherry-pick release-1.9

@openshift-cherrypick-robot
Copy link
Contributor

@zdrapela: new pull request created: #4105

Details

In response to this:

/cherry-pick release-1.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants