Skip to content

Conversation

@mashraf-222
Copy link
Contributor

@mashraf-222 mashraf-222 commented Oct 28, 2025

User description

https://linear.app/codeflash-ai/issue/CF-784/codeflash-does-not-exit-on-invalid-api-key

When we have invalid API Key optimization should be stopped, even though it continue. in this PR we are validating the API key user info before lunching optimizer.


PR Type

Bug fix, Enhancement


Description

  • Validate API key before optimizer starts

  • Add clear error messages and exit codes

  • Handle outdated CLI via min_version check

  • Improve cleanup on failures and interrupts


Diagram Walkthrough

flowchart LR
  A["CLI run_with_args"] -- "validate_api_key()" --> B["CF API /cli-get-user"]
  B -- "200 OK" --> C["Proceed: initialize Optimizer"]
  B -- "403/Non-200" --> D["Raise OSError with guidance"]
  B -- "min_version > __version__" --> E["Raise OSError: update CLI"]
  C -- "run()" --> F["Optimization flow"]
  A -- "KeyboardInterrupt" --> G["Cleanup and SystemExit(0)"]
  A -- "Errors" --> H["Log, cleanup if optimizer, re-raise"]
Loading

File Walkthrough

Relevant files
Enhancement
cfapi.py
Introduce API key validation and version gating                   

codeflash/api/cfapi.py

  • Add validate_api_key helper using /cli-get-user.
  • Return actionable errors for 403 and non-200 responses.
  • Check min_version and require CLI upgrade.
  • Robust exception wrapping for validation failures.
+64/-0   
Bug fix
optimizer.py
Early key validation and safer failure handling                   

codeflash/optimization/optimizer.py

  • Call validate_api_key() before optimizer initialization.
  • Improve KeyboardInterrupt handling with explicit exit code.
  • Add error logging and cleanup on failures.
+16/-3   

@CLAassistant
Copy link

CLAassistant commented Oct 28, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ mashraf-222
❌ mohamedashrraf222
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Possible Issue

Accessing response.status_code assumes response is not None; ensure make_cfapi_request always returns a response object when suppress_errors=True, or guard against None to avoid AttributeError.

if response.status_code == 403:
    msg = (
        "Invalid Codeflash API key. The API key you provided is not valid.\n"
        "Please generate a new one at https://app.codeflash.ai/app/apikeys ,\n"
        "then set it as a CODEFLASH_API_KEY environment variable.\n"
        "For more information, refer to the documentation at "
        "https://docs.codeflash.ai/getting-started/codeflash-github-actions#add-your-api-key-to-your-repository-secrets."
    )
    raise OSError(msg)
elif response.status_code != 200:
    msg = (
Robustness

Version comparison uses version.parse without importing/ensuring availability here; confirm import exists in this module or add it to avoid NameError at runtime.

if min_version and version.parse(min_version) > version.parse(__version__):
    msg = "Your Codeflash CLI version is outdated. Please update to the latest version using `pip install --upgrade codeflash`."
    raise OSError(msg)
Behavior Change

Exiting with SystemExit(0) on KeyboardInterrupt may mask interruption as success; consider non-zero code or align with project conventions for CI signaling.

    raise SystemExit(0) from None
except (OSError, Exception) as e:

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Reject blank API keys

Treat empty or whitespace-only values as missing to avoid passing an invalid blank
key. Strip the value and re-check before proceeding.

codeflash/api/cfapi.py [132-134]

 if not api_key:
     api_key = get_codeflash_api_key()
+if api_key is not None and isinstance(api_key, str):
+    api_key = api_key.strip()
+if not api_key:
+    raise OSError("Missing Codeflash API key. Please set CODEFLASH_API_KEY.")
Suggestion importance[1-10]: 7

__

Why: Treating whitespace-only API keys as missing is a sensible validation hardening aligned with the new function's purpose; it accurately extends the existing check without conflicting with the PR. Impact is moderate as it improves robustness but not a critical bug fix.

Medium
Guard against None responses

Avoid assuming response is non-null; transient network errors may yield None. Guard
against None and missing attributes to prevent attribute errors and provide a clear
message.

codeflash/api/cfapi.py [137-160]

 response = make_cfapi_request(
-    endpoint="/cli-get-user", 
-    method="GET", 
-    extra_headers={"cli_version": __version__}, 
+    endpoint="/cli-get-user",
+    method="GET",
+    extra_headers={"cli_version": __version__},
     api_key=api_key,
-    suppress_errors=True  # Don't log errors yet, we'll handle it below
+    suppress_errors=True
 )
+
+if response is None or getattr(response, "status_code", None) is None:
+    raise OSError("Failed to validate API key: no response from Codeflash API. Please check your network and try again.")
 
 if response.status_code == 403:
     msg = (
         "Invalid Codeflash API key. The API key you provided is not valid.\n"
         "Please generate a new one at https://app.codeflash.ai/app/apikeys ,\n"
         "then set it as a CODEFLASH_API_KEY environment variable.\n"
         "For more information, refer to the documentation at "
         "https://docs.codeflash.ai/getting-started/codeflash-github-actions#add-your-api-key-to-your-repository-secrets."
     )
     raise OSError(msg)
-elif response.status_code != 200:
+if response.status_code != 200:
     msg = (
         f"Failed to validate API key with Codeflash API (status {response.status_code}).\n"
         "Please verify your API key is correct.\n"
         "You can generate a new one at https://app.codeflash.ai/app/apikeys"
     )
     raise OSError(msg)
Suggestion importance[1-10]: 6

__

Why: Adding a guard for a None response prevents attribute errors and yields clearer errors; however, it's speculative since make_cfapi_request likely returns a response object. Still a reasonable defensive check with modest impact.

Low
General
Cleanly exit on OSError

Avoid catching Exception and re-raising bare, which can double-log and mask exit
codes. Handle OSError explicitly with a non-zero exit, and let other exceptions
propagate once without extra wrapping.

codeflash/optimization/optimizer.py [501-516]

 except KeyboardInterrupt:
     logger.warning("Keyboard interrupt received. Cleaning up and exiting, please wait…")
     if optimizer:
         optimizer.cleanup_temporary_paths()
     raise SystemExit(0) from None
-except (OSError, Exception) as e:
-    # Log the error if we have an optimizer instance with cleanup
+except OSError as e:
     if optimizer:
         logger.error(f"Error during optimization: {e}")
         try:
             optimizer.cleanup_temporary_paths()
         except Exception:
             pass
     else:
         logger.error(f"Error initializing optimizer: {e}")
-    raise
+    raise SystemExit(1) from None
Suggestion importance[1-10]: 5

__

Why: Narrowing the except clause to OSError can reduce double-logging and provide a consistent non-zero exit, but it changes current behavior that re-raises all exceptions and may hide useful diagnostics. It's a stylistic/runtime behavior change with limited, context-dependent benefit.

Low

"Please generate a new one at https://app.codeflash.ai/app/apikeys ,\n"
"then set it as a CODEFLASH_API_KEY environment variable.\n"
"For more information, refer to the documentation at "
"https://docs.codeflash.ai/getting-started/codeflash-github-actions#add-your-api-key-to-your-repository-secrets."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

404 error and remove the dot at the end of the url

Copy link
Contributor Author

@mashraf-222 mashraf-222 Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't have this section yet, I was thinking to add it, or should I just remove this placeholder and keep https://app.codeflash.ai/app/apikeys only ?

return None


def validate_api_key(api_key: Optional[str] = None) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont duplicate this call. it is already happening. please handle it there

@mashraf-222 mashraf-222 force-pushed the fix/invalid-api-key/stop-optimization branch from 06e16ea to 2f001bb Compare October 29, 2025 22:38
Comment on lines 124 to 132
def validate_api_key() -> None:
"""Validate the API key by making a request to the /cfapi/cli-get-user endpoint.
Raises SystemExit if the API key is invalid (403) or missing.
This should be called early in the CLI flow before starting optimization.
"""
logger.debug("validate_api_key: Starting API key validation")
api_key = get_codeflash_api_key()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

@KRRT7
Copy link
Contributor

KRRT7 commented Oct 29, 2025

oops, sorry, I misunderstood what was happening, LGTM now

@KRRT7
Copy link
Contributor

KRRT7 commented Oct 29, 2025

I don't think we need to always validate the API key, we can validate just for non-init commands

@mashraf-222 mashraf-222 force-pushed the fix/invalid-api-key/stop-optimization branch from ae6d1f8 to 7eb6a38 Compare October 29, 2025 23:24
KRRT7
KRRT7 previously approved these changes Oct 29, 2025
@@ -1,3 +1,5 @@
"""Module for interacting with the Codeflash API."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this comment, users are not expected with the API

@KRRT7 KRRT7 merged commit 376ca0d into main Oct 31, 2025
20 of 22 checks passed
@KRRT7 KRRT7 deleted the fix/invalid-api-key/stop-optimization branch October 31, 2025 01:02
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.

6 participants