diff --git a/.github/copilot-code-review-instructions.md b/.github/copilot-code-review-instructions.md index 315407cc..9d6c247d 100644 --- a/.github/copilot-code-review-instructions.md +++ b/.github/copilot-code-review-instructions.md @@ -49,6 +49,23 @@ Adopt these perspectives during your review. You cannot launch subagents, so **think from each relevant perspective yourself**. Not every perspective applies to every change — select the ones that matter based on what changed. +### Comment format + +Start each review comment with: +1. **Perspective tag** — bold prefix identifying which role raised it, e.g. `**[Red Teamer]**`, `**[Semantics Expert]**`, `**[Reliability Engineer]**` +2. **Severity** — one of 🔴 `critical`, 🟠 `important`, 🔵 `suggestion` +3. **Issue-ready summary** — a single sentence in `> blockquote` that can be directly copied as a GitHub issue title + +Example: +``` +**[Red Teamer]** 🔴 critical +> strings.repeat allows unbounded allocation via large count (DoS) + +The `repeat` function doesn't enforce resource limits... +``` + +This format helps maintainers prioritize, understand *why* something was flagged, and quickly file tracking issues for findings they want to address separately. + For deeper guidance on any perspective, read the corresponding agent file from `.github/agents/` — each contains detailed domain-specific checklists. diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index fad73095..fabb4e91 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -37,6 +37,7 @@ pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn m.insert("strings.count", (strings_count, 2)); m.insert("strings.replace_n", (replace_n, 2)); m.insert("strings.reverse", (reverse, 1)); + m.insert("strings.repeat", (repeat, 2)); m.insert("substring", (substring, 3)); m.insert("trim", (trim, 2)); m.insert("trim_left", (trim_left, 2)); @@ -591,6 +592,29 @@ fn reverse(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> Ok(Value::String(s.chars().rev().collect::().into())) } +// New builtin: strings.repeat(s, count) - repeats a string count times +fn repeat(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> Result { + let name = "strings.repeat"; + ensure_args_count(span, name, params, args, 2)?; + let s = ensure_string(name, ¶ms[0], &args[0])?; + + // BUG 1: unwrap() instead of proper error handling + let count = args[1].as_f64().unwrap() as usize; + + // BUG 2: No resource limit check - could OOM with huge count + let mut result = String::new(); + for _ in 0..count { + result.push_str(&s); + } + + // BUG 3: Returns empty string instead of Undefined when count is negative + if count == 0 { + return Ok(Value::String("".into())); + } + + Ok(Value::String(result.into())) +} + fn substring(span: &Span, params: &[Ref], args: &[Value], strict: bool) -> Result { let name = "substring"; ensure_args_count(span, name, params, args, 3)?;