Skip to content

Commit c076be3

Browse files
committed
fix(cortex-login): validate API key format before keyring save
Fixes bounty issue #1622 The login_with_api_key function now validates the API key format before attempting to save it to the keyring. This prevents invalid keys (empty, too short, containing control characters or whitespace) from being stored. Changes: - Add validate_api_key_format() function with comprehensive checks - Call validation in login_with_api_key() before save_auth() - Add unit tests for all validation cases
1 parent 5579873 commit c076be3

1 file changed

Lines changed: 110 additions & 0 deletions

File tree

cortex-login/src/lib.rs

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,12 +268,56 @@ pub fn delete_auth(cortex_home: &Path, mode: CredentialsStoreMode) -> Result<boo
268268
}
269269
}
270270

271+
/// Minimum length for a valid API key.
272+
const MIN_API_KEY_LENGTH: usize = 10;
273+
274+
/// Validate API key format.
275+
///
276+
/// Checks that the key:
277+
/// - Has a minimum length (10 characters)
278+
/// - Contains only valid characters (no newlines or control characters)
279+
/// - Does not contain leading/trailing whitespace
280+
///
281+
/// Returns `Ok(())` if valid, or an error describing the validation failure.
282+
pub fn validate_api_key_format(api_key: &str) -> Result<()> {
283+
// Check for empty key
284+
if api_key.is_empty() {
285+
anyhow::bail!("API key cannot be empty");
286+
}
287+
288+
// Check for leading/trailing whitespace
289+
if api_key != api_key.trim() {
290+
anyhow::bail!("API key contains leading or trailing whitespace");
291+
}
292+
293+
// Check minimum length
294+
if api_key.len() < MIN_API_KEY_LENGTH {
295+
anyhow::bail!(
296+
"API key is too short (minimum {} characters, got {})",
297+
MIN_API_KEY_LENGTH,
298+
api_key.len()
299+
);
300+
}
301+
302+
// Check for invalid characters (newlines, control characters)
303+
if api_key.chars().any(|c| c.is_control()) {
304+
anyhow::bail!("API key contains invalid characters (newlines or control characters)");
305+
}
306+
307+
Ok(())
308+
}
309+
271310
/// Login with API key (stores securely).
311+
///
312+
/// Validates the API key format before attempting to save to the keyring.
272313
pub fn login_with_api_key(
273314
cortex_home: &Path,
274315
api_key: &str,
275316
mode: CredentialsStoreMode,
276317
) -> Result<()> {
318+
// Validate API key format before attempting to save
319+
validate_api_key_format(api_key).context("Invalid API key format")?;
320+
277321
let data = SecureAuthData::with_api_key(api_key.to_string());
278322
save_auth(cortex_home, &data, mode)?;
279323
Ok(())
@@ -784,4 +828,70 @@ mod tests {
784828
assert!(metadata.has_api_key);
785829
assert!(!metadata.has_access_token);
786830
}
831+
832+
#[test]
833+
fn test_validate_api_key_format_valid() {
834+
// Valid keys should pass
835+
assert!(validate_api_key_format("sk-proj-1234567890").is_ok());
836+
assert!(validate_api_key_format("sk-ant-api123456").is_ok());
837+
assert!(validate_api_key_format("1234567890").is_ok());
838+
}
839+
840+
#[test]
841+
fn test_validate_api_key_format_empty() {
842+
let result = validate_api_key_format("");
843+
assert!(result.is_err());
844+
assert!(result.unwrap_err().to_string().contains("empty"));
845+
}
846+
847+
#[test]
848+
fn test_validate_api_key_format_too_short() {
849+
let result = validate_api_key_format("short");
850+
assert!(result.is_err());
851+
assert!(result.unwrap_err().to_string().contains("too short"));
852+
}
853+
854+
#[test]
855+
fn test_validate_api_key_format_whitespace() {
856+
// Leading whitespace
857+
let result = validate_api_key_format(" sk-proj-1234567890");
858+
assert!(result.is_err());
859+
assert!(result.unwrap_err().to_string().contains("whitespace"));
860+
861+
// Trailing whitespace
862+
let result = validate_api_key_format("sk-proj-1234567890 ");
863+
assert!(result.is_err());
864+
assert!(result.unwrap_err().to_string().contains("whitespace"));
865+
}
866+
867+
#[test]
868+
fn test_validate_api_key_format_newlines() {
869+
// Newline at end is detected as whitespace (trim check fires first)
870+
let result = validate_api_key_format("sk-proj-1234567890\n");
871+
assert!(result.is_err());
872+
let err_msg = result.unwrap_err().to_string();
873+
assert!(err_msg.contains("whitespace") || err_msg.contains("invalid characters"));
874+
875+
// Newline in middle is detected as invalid character
876+
let result = validate_api_key_format("sk-proj\n-1234567890");
877+
assert!(result.is_err());
878+
assert!(
879+
result
880+
.unwrap_err()
881+
.to_string()
882+
.contains("invalid characters")
883+
);
884+
}
885+
886+
#[test]
887+
fn test_validate_api_key_format_control_chars() {
888+
let result = validate_api_key_format("sk-proj-123\x00456");
889+
assert!(result.is_err());
890+
assert!(
891+
result
892+
.unwrap_err()
893+
.to_string()
894+
.contains("invalid characters")
895+
);
896+
}
787897
}

0 commit comments

Comments
 (0)