Skip to content

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Sep 17, 2025

PR Type

Enhancement, Bug fix


Description

  • Prefer env var, fallback to shell config

  • Persist env API key into shell config

  • Add debug logs around persistence

  • Remove LSP-specific behavior


Diagram Walkthrough

flowchart LR
  A["Get CODEFLASH_API_KEY from env"] -- "exists?" --> B["Use env API key"]
  A -- "missing" --> C["Read from shell config"]
  B -- "shell lacks key" --> D["Save env key to shell rc"]
  C -- "found" --> E["Use shell API key"]
  D -- "success/failure logged" --> B
Loading

File Walkthrough

Relevant files
Enhancement
env_utils.py
Env-first API key retrieval with persistence                         

codeflash/code_utils/env_utils.py

  • Switch precedence: env var before shell config.
  • Auto-save env API key to shell rc if absent.
  • Add debug logging with unwrap on success.
  • Remove LSP-based conditional logic and import.
+16/-8   

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


saga4 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
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
🔒 Security concerns

Sensitive information exposure:
Ensure that the debug message after saving the API key cannot include the key itself. While the current code logs result.unwrap(), confirm that this does not contain the secret; if it could, replace with a generic success message or sanitize the output. Additionally, logging raw exception messages during save could inadvertently include sensitive paths or content—consider sanitizing or reducing verbosity in non-debug modes.

⚡ Recommended focus areas for review

Logging Content

Debug logs include the unwrapped result of saving the API key, which might reveal file paths or details not necessary for debugging; consider avoiding logging sensitive or overly specific information and ensure the key itself can never be logged.

    if is_successful(result):
        logger.debug(f"Automatically saved API key from environment to shell config: {result.unwrap()}")
except Exception as e:
    logger.debug(f"Failed to automatically save API key to shell config: {e}")
Error Handling

Broad exception catch during persistence may hide actionable errors; consider narrowing exceptions or adding at least the exception type and result state to help diagnose failures without leaking sensitive data.

    from codeflash.either import is_successful
    result = save_api_key_to_rc(env_api_key)
    if is_successful(result):
        logger.debug(f"Automatically saved API key from environment to shell config: {result.unwrap()}")
except Exception as e:
    logger.debug(f"Failed to automatically save API key to shell config: {e}")
Caching Behavior

Function is cached with lru_cache; changes to env var or shell config after first call won't be reflected. Verify this is intended given new persistence logic and debug flows.

@lru_cache(maxsize=1)
def get_codeflash_api_key() -> str:
    # Check environment variable first
    env_api_key = os.environ.get("CODEFLASH_API_KEY")
    shell_api_key = read_api_key_from_shell_config()

    # If we have an env var but it's not in shell config, save it for persistence
    if env_api_key and not shell_api_key:
        try:
            from codeflash.either import is_successful
            result = save_api_key_to_rc(env_api_key)
            if is_successful(result):
                logger.debug(f"Automatically saved API key from environment to shell config: {result.unwrap()}")
        except Exception as e:
            logger.debug(f"Failed to automatically save API key to shell config: {e}")

    api_key = env_api_key or shell_api_key

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Prevent sensitive data leakage

Restrict logging to avoid exposing the API key or sensitive details. Log a generic
success/failure message without unwrapping or interpolating values that may contain
secrets.

codeflash/code_utils/env_utils.py [42-49]

 if env_api_key and not shell_api_key:
     try:
         from codeflash.either import is_successful
         result = save_api_key_to_rc(env_api_key)
         if is_successful(result):
-            logger.debug(f"Automatically saved API key from environment to shell config: {result.unwrap()}")
-    except Exception as e:
-        logger.debug(f"Failed to automatically save API key to shell config: {e}")
+            logger.debug("Automatically saved API key from environment to shell config.")
+    except Exception:
+        logger.debug("Failed to automatically save API key to shell config.", exc_info=False)
Suggestion importance[1-10]: 8

__

Why: Correctly removes logging of potentially sensitive values and avoids exposing exception details; this is a meaningful security improvement aligned with the new code block.

Medium
Possible issue
Restore stable precedence order

Preserve previous behavior where shell config can take precedence in certain
contexts. Consider preferring shell_api_key when present to avoid transient env
overrides causing unexpected key usage.

codeflash/code_utils/env_utils.py [37-52]

 env_api_key = os.environ.get("CODEFLASH_API_KEY")
 shell_api_key = read_api_key_from_shell_config()
 ...
-api_key = env_api_key or shell_api_key
+api_key = shell_api_key or env_api_key
Suggestion importance[1-10]: 5

__

Why: Suggestion is reasonable but opinionated; the PR explicitly changes precedence to env-first and also persists env to shell, so reverting precedence may contradict intended behavior.

Low
General
Eliminate runtime import in path

Avoid importing inside the hot path to prevent repeated import overhead and
potential circular import issues. Move is_successful import to module scope or use a
simple truthy check on result if supported.

codeflash/code_utils/env_utils.py [42-49]

+# at module top:
+# from codeflash.either import is_successful
+...
 if env_api_key and not shell_api_key:
     try:
-        from codeflash.either import is_successful
         result = save_api_key_to_rc(env_api_key)
         if is_successful(result):
-            logger.debug(f"Automatically saved API key from environment to shell config: {result.unwrap()}")
-    except Exception as e:
-        logger.debug(f"Failed to automatically save API key to shell config: {e}")
+            logger.debug("Automatically saved API key from environment to shell config.")
+    except Exception:
+        logger.debug("Failed to automatically save API key to shell config.", exc_info=False)
Suggestion importance[1-10]: 4

__

Why: Avoiding in-function imports can marginally improve performance/readability, but impact is minor and may risk import cycles; also caching via lru_cache reduces frequency of this path.

Low

@Saga4 Saga4 enabled auto-merge (squash) September 18, 2025 05:49
@Saga4 Saga4 disabled auto-merge September 18, 2025 06:11
@Saga4 Saga4 merged commit 48e88b7 into main Sep 18, 2025
17 of 21 checks passed
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