Skip to content

Commit 389a319

Browse files
committed
fix(cortex-cli): validate PR numbers locally before API calls
Fixes bounty issue #1611 Added local validation for PR numbers in the pr command to prevent wasting GitHub API rate limit quota on obviously invalid PR numbers. The validation rejects zero and numbers exceeding 10 million, which are unlikely to be valid PR numbers.
1 parent 8f839ec commit 389a319

File tree

1 file changed

+63
-0
lines changed

1 file changed

+63
-0
lines changed

cortex-cli/src/pr_cmd.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,27 @@ fn validate_refspec(refspec: &str) -> Result<()> {
5353
Ok(())
5454
}
5555

56+
/// Maximum reasonable PR number threshold.
57+
/// GitHub PR numbers are sequential and unlikely to exceed this value.
58+
/// This prevents wasting API calls on obviously invalid PR numbers.
59+
const MAX_REASONABLE_PR_NUMBER: u64 = 10_000_000;
60+
61+
/// Validates that a PR number is within a reasonable range before making API calls.
62+
/// This prevents wasting API rate limit quota on obviously invalid PR numbers.
63+
fn validate_pr_number(pr_number: u64) -> Result<()> {
64+
if pr_number == 0 {
65+
bail!("Invalid PR number: PR numbers must be greater than 0.");
66+
}
67+
if pr_number > MAX_REASONABLE_PR_NUMBER {
68+
bail!(
69+
"Invalid PR number: {} exceeds the maximum reasonable PR number ({}).",
70+
pr_number,
71+
MAX_REASONABLE_PR_NUMBER
72+
);
73+
}
74+
Ok(())
75+
}
76+
5677
/// Pull Request CLI.
5778
#[derive(Debug, Parser)]
5879
pub struct PrCli {
@@ -90,6 +111,9 @@ async fn run_pr_checkout(args: PrCli) -> Result<()> {
90111
let repo_path = args.path.unwrap_or_else(|| PathBuf::from("."));
91112
let pr_number = args.number;
92113

114+
// Validate PR number locally before making any API calls
115+
validate_pr_number(pr_number)?;
116+
93117
// Change to repo directory
94118
std::env::set_current_dir(&repo_path)
95119
.with_context(|| format!("Failed to change to directory: {}", repo_path.display()))?;
@@ -342,4 +366,43 @@ mod tests {
342366
assert_eq!(owner, "Cortex-ai");
343367
assert_eq!(repo, "Cortex");
344368
}
369+
370+
#[test]
371+
fn test_validate_pr_number_valid() {
372+
assert!(validate_pr_number(1).is_ok());
373+
assert!(validate_pr_number(100).is_ok());
374+
assert!(validate_pr_number(12345).is_ok());
375+
assert!(validate_pr_number(MAX_REASONABLE_PR_NUMBER).is_ok());
376+
}
377+
378+
#[test]
379+
fn test_validate_pr_number_zero() {
380+
let result = validate_pr_number(0);
381+
assert!(result.is_err());
382+
assert!(
383+
result
384+
.unwrap_err()
385+
.to_string()
386+
.contains("must be greater than 0")
387+
);
388+
}
389+
390+
#[test]
391+
fn test_validate_pr_number_too_large() {
392+
let result = validate_pr_number(999_999_999_999);
393+
assert!(result.is_err());
394+
assert!(
395+
result
396+
.unwrap_err()
397+
.to_string()
398+
.contains("exceeds the maximum")
399+
);
400+
}
401+
402+
#[test]
403+
fn test_validate_pr_number_boundary() {
404+
// Just above the limit should fail
405+
let result = validate_pr_number(MAX_REASONABLE_PR_NUMBER + 1);
406+
assert!(result.is_err());
407+
}
345408
}

0 commit comments

Comments
 (0)