Skip to content

fix(id): reject malformed REPL flags #3092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dezren39 merged 3 commits into from
Nov 2, 2026
Merged

fix(id): reject malformed REPL flags #3092

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f5f52a
fix(id): reject malformed REPL flags
dezren39 Oct 31, 2026
4358a32
style(id): rustfmt the adapted peek test assertion
dezren39 Nov 2, 2026
763816a
fix(id): let --exclude take hyphen-prefixed patterns (#2274)
dezren39 Nov 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 118 additions & 26 deletions pkgs/id/src/repl/runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,31 @@ impl FindArgs<'_> {
/// - `--last [N]`: Return only last N matches (default 1)
/// - `--count`: Print count instead of matches
/// - `--exclude PATTERN`: Exclude matches containing pattern (repeatable)
fn required_value<'a>(rest: &[&'a str], i: &mut usize) -> Result<&'a str> {
let flag = rest[*i];
let value = rest
.get(*i + 1)
.copied()
.filter(|value| !value.is_empty() && (!value.starts_with('-') || *value == "-"))
Comment thread
dezren39 marked this conversation as resolved.
Resolved
.ok_or_else(|| anyhow::anyhow!("{flag} requires a value"))?;
*i += 1;
Ok(value)
}

/// Like `required_value`, but for `--exclude`: exclusion patterns are
/// data, and a pattern may legitimately start with a hyphen (e.g.
/// `--exclude -draft`), so any non-empty next token is accepted.
fn required_pattern<'a>(rest: &[&'a str], i: &mut usize) -> Result<&'a str> {
let flag = rest[*i];
let value = rest
.get(*i + 1)
.copied()
.filter(|value| !value.is_empty())
.ok_or_else(|| anyhow::anyhow!("{flag} requires a value"))?;
*i += 1;
Ok(value)
}

fn parse_find_args<'a>(rest: &[&'a str], default_format: &'a str) -> Result<FindArgs<'a>> {
let mut args = FindArgs {
queries: Vec::new(),
Expand Down Expand Up @@ -682,22 +707,23 @@ fn parse_find_args<'a>(rest: &[&'a str], default_format: &'a str) -> Result<Find
} else if arg == "--file" {
args.to_file = true;
} else if let Some(path) = arg.strip_prefix('>') {
anyhow::ensure!(
!path.is_empty(),
"redirect > requires a non-empty output path"
);
args.output_file = Some(path);
args.to_file = true;
} else if arg == "-o" || arg == "--output" {
args.output_file = Some(required_value(rest, &mut i)?);
args.to_file = true;
} else if arg == "--dir" {
if i + 1 < rest.len() {
args.dir = Some(rest[i + 1]);
i += 1;
}
args.dir = Some(required_value(rest, &mut i)?);
} else if arg == "--format" {
if i + 1 < rest.len() {
let value = rest[i + 1];
if matches!(value, "tag" | "group" | "union") {
args.format = value;
} else {
args.format_error = Some(value);
}
i += 1;
let value = required_value(rest, &mut i)?;
if matches!(value, "tag" | "group" | "union") {
args.format = value;
} else {
args.format_error = Some(value);
}
} else if arg == "--tag" {
args.format = "tag";
Expand Down Expand Up @@ -742,11 +768,10 @@ fn parse_find_args<'a>(rest: &[&'a str], default_format: &'a str) -> Result<Find
} else if arg == "--count" {
args.count = true;
} else if arg == "--exclude" {
if i + 1 < rest.len() {
args.exclude.push(rest[i + 1]);
i += 1;
}
} else if !arg.starts_with('-') {
args.exclude.push(required_pattern(rest, &mut i)?);
} else if arg.starts_with('-') {
anyhow::bail!("unknown flag: {arg}");
} else {
args.queries.push(arg);
}
i += 1;
Expand Down Expand Up @@ -1282,13 +1307,14 @@ fn parse_peek_args<'a>(rest: &[&'a str]) -> Result<(FindArgs<'a>, PeekOptions)>
} else if arg == "--all" {
find_args.all = true;
} else if arg == "-o" || arg == "--output" {
if i + 1 < rest.len() {
find_args.output_file = Some(rest[i + 1]);
i += 1;
}
find_args.output_file = Some(required_value(rest, &mut i)?);
} else if let Some(path) = arg.strip_prefix('>') {
// Mirror parse_find_args: `>file` redirects the preview to
// a file (the peek handler already honors output_file).
anyhow::ensure!(
!path.is_empty(),
"redirect > requires a non-empty output path"
);
find_args.output_file = Some(path);
find_args.to_file = true;
} else if arg == "--first" {
Expand Down Expand Up @@ -1324,10 +1350,7 @@ fn parse_peek_args<'a>(rest: &[&'a str]) -> Result<(FindArgs<'a>, PeekOptions)>
find_args.last = Some(1);
}
} else if arg == "--exclude" {
if i + 1 < rest.len() {
find_args.exclude.push(rest[i + 1]);
i += 1;
}
find_args.exclude.push(required_pattern(rest, &mut i)?);
} else if arg == "--lines" || arg == "-n" {
// Same redirection exclusion: `--lines >out.txt` keeps
// the default line count and lets the `>` branch run.
Expand All @@ -1352,7 +1375,9 @@ fn parse_peek_args<'a>(rest: &[&'a str]) -> Result<(FindArgs<'a>, PeekOptions)>
peek_opts.words = true;
} else if arg == "--quiet" || arg == "-q" {
peek_opts.quiet = true;
} else if !arg.starts_with('-') {
} else if arg.starts_with('-') {
anyhow::bail!("unknown flag: {arg}");
} else {
find_args.queries.push(arg);
}
i += 1;
Expand Down Expand Up @@ -1773,6 +1798,73 @@ mod tests {
assert_eq!(args.format, "union");
}

#[test]
fn test_find_rejects_malformed_flags_and_redirects() {
for (tokens, expected) in [
(vec!["report", "--bogus"], "--bogus"),
(vec!["report", "--fist", "2"], "--fist"),
(vec!["report", "--dir"], "--dir"),
(vec!["report", "--exclude"], "--exclude"),
(vec!["report", "--format"], "--format"),
(vec!["report", "-o"], "-o"),
(vec!["report", "--output", "--all"], "--output"),
(vec!["report", ">"], ">"),
] {
let error = parse_find_args(&tokens, "union").err().unwrap().to_string();
assert!(error.contains(expected), "{tokens:?}: {error}");
}
// Invalid --format values are carried as format_error for the
// handlers to report (main's 3488a51ac design, which superseded
// this branch's original immediate parse error on rebase).
let args = parse_find_args(&["report", "--format", "yaml"], "union").unwrap();
assert_eq!(args.format_error, Some("yaml"));
assert_eq!(
parse_find_args(&["report", "-o", "out"], "union")
.unwrap()
.output_file,
Some("out")
);
assert_eq!(
parse_find_args(&["report", "--exclude", "-draft"], "union")
.unwrap()
.exclude,
vec!["-draft"]
);
}

#[test]
fn test_peek_rejects_malformed_flags_and_missing_values() {
for (tokens, expected) in [
(vec!["report", "--bogus"], "--bogus"),
(vec!["report", "--fist", "2"], "--fist"),
(vec!["report", "-o"], "-o"),
(vec!["report", "--output"], "--output"),
(vec!["report", "--exclude"], "--exclude"),
(vec!["report", "--lines", "wrong"], "wrong"),
(vec!["report", ">"], ">"),
] {
let error = parse_peek_args(&tokens).err().unwrap().to_string();
assert!(error.contains(expected), "{tokens:?}: {error}");
}
// A bare --lines/-n keeps the default line count, matching the
// merged-first set's bare --first/--last default design (#4072).
assert_eq!(
parse_peek_args(&["report", "--exclude", "-draft"])
.unwrap()
.0
.exclude,
vec!["-draft"]
);
assert_eq!(parse_peek_args(&["report", "--lines"]).unwrap().1.lines, 5);
assert_eq!(
parse_peek_args(&["report", "-n", "--quiet"])
.unwrap()
.1
.lines,
5
);
}

#[test]
fn test_parse_find_args_with_flags() {
let rest = vec!["query", "--name", "--all", "--format", "tag"];
Expand Down
Loading