Skip to content

Commit da3703f

Browse files
committed
fix(cli): validate --fail-on severity and improve changed-file matching
- Reject unknown severity values in --fail-on instead of silently disabling the threshold (e.g. --fail-on crtical now errors) - Require directory component in suffix path matching to prevent bare filenames from matching across unrelated directories - Warn on stderr when source files cannot be read for suppression comments instead of silently skipping all suppressions fix(liquibase): report errors for missing required XML attributes
1 parent 53d83b1 commit da3703f

2 files changed

Lines changed: 58 additions & 9 deletions

File tree

src/input/liquibase_xml.rs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,12 @@ fn handle_start_tag(
263263
match tag_name {
264264
"sql" => Ok(ParseState::InSqlTag(cs, String::new())),
265265
"createTable" => {
266-
let table_name = get_attr(attrs, "tableName").unwrap_or_default();
266+
let Some(table_name) = get_attr(attrs, "tableName") else {
267+
eprintln!(
268+
"Warning: <createTable> missing required 'tableName' attribute, skipping"
269+
);
270+
return Ok(ParseState::InChangeSet(cs));
271+
};
267272
let schema_name = get_attr(attrs, "schemaName");
268273
Ok(ParseState::InCreateTable(
269274
cs,
@@ -275,7 +280,12 @@ fn handle_start_tag(
275280
))
276281
}
277282
"addColumn" => {
278-
let table_name = get_attr(attrs, "tableName").unwrap_or_default();
283+
let Some(table_name) = get_attr(attrs, "tableName") else {
284+
eprintln!(
285+
"Warning: <addColumn> missing required 'tableName' attribute, skipping"
286+
);
287+
return Ok(ParseState::InChangeSet(cs));
288+
};
279289
let schema_name = get_attr(attrs, "schemaName");
280290
Ok(ParseState::InAddColumn(
281291
cs,
@@ -288,7 +298,12 @@ fn handle_start_tag(
288298
}
289299
"createIndex" => {
290300
let index_name = get_attr(attrs, "indexName").unwrap_or_default();
291-
let table_name = get_attr(attrs, "tableName").unwrap_or_default();
301+
let Some(table_name) = get_attr(attrs, "tableName") else {
302+
eprintln!(
303+
"Warning: <createIndex> missing required 'tableName' attribute, skipping"
304+
);
305+
return Ok(ParseState::InChangeSet(cs));
306+
};
292307
let schema_name = get_attr(attrs, "schemaName");
293308
let unique = get_attr(attrs, "unique")
294309
.map(|v| v == "true")
@@ -305,7 +320,12 @@ fn handle_start_tag(
305320
))
306321
}
307322
"dropTable" => {
308-
let table_name = get_attr(attrs, "tableName").unwrap_or_default();
323+
let Some(table_name) = get_attr(attrs, "tableName") else {
324+
eprintln!(
325+
"Warning: <dropTable> missing required 'tableName' attribute, skipping"
326+
);
327+
return Ok(ParseState::InChangeSet(cs));
328+
};
309329
let schema_name = get_attr(attrs, "schemaName");
310330
let qualified = qualify_name(&schema_name, &table_name);
311331
cs.sql_parts.push(format!("DROP TABLE {};", qualified));
@@ -324,6 +344,12 @@ fn handle_start_tag(
324344
Ok(ParseState::InChangeSet(cs))
325345
}
326346
"addPrimaryKey" => {
347+
if get_attr(attrs, "tableName").is_none() {
348+
eprintln!(
349+
"Warning: <addPrimaryKey> missing required 'tableName' attribute, skipping"
350+
);
351+
return Ok(ParseState::InChangeSet(cs));
352+
}
327353
let sql = generate_add_pk_sql(attrs);
328354
cs.sql_parts.push(sql);
329355
Ok(ParseState::InChangeSet(cs))

src/main.rs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,12 @@ fn run(args: Args) -> Result<bool> {
121121
.unwrap_or_else(|_| unit.source_file.clone());
122122
changed_files_set.contains(&canonical)
123123
|| changed_files_set.contains(&unit.source_file)
124-
|| changed_files_set
125-
.iter()
126-
.any(|cf| cf.ends_with(&unit.source_file) || unit.source_file.ends_with(cf))
124+
|| changed_files_set.iter().any(|cf| {
125+
// Only allow suffix matching when the shorter path includes a directory
126+
// component, to prevent bare filenames from matching across directories.
127+
(cf.ends_with(&unit.source_file) && unit.source_file.components().count() > 1)
128+
|| (unit.source_file.ends_with(cf) && cf.components().count() > 1)
129+
})
127130
};
128131

129132
if is_changed {
@@ -168,7 +171,17 @@ fn run(args: Args) -> Result<bool> {
168171

169172
// Parse suppressions from source file and filter findings.
170173
// Read the raw SQL source for suppression comments.
171-
let source = std::fs::read_to_string(&unit.source_file).unwrap_or_default();
174+
let source = match std::fs::read_to_string(&unit.source_file) {
175+
Ok(s) => s,
176+
Err(e) => {
177+
eprintln!(
178+
"Warning: could not read '{}' for suppression comments: {}",
179+
unit.source_file.display(),
180+
e
181+
);
182+
String::new()
183+
}
184+
};
172185
let suppressions = parse_suppressions(&source);
173186

174187
unit_findings.retain(|f| !suppressions.is_suppressed(&f.rule_id, f.start_line));
@@ -224,7 +237,17 @@ fn run(args: Args) -> Result<bool> {
224237
eprintln!("pg-migration-lint: {} finding(s)", all_findings.len());
225238

226239
let fail_on_str = args.fail_on.as_deref().unwrap_or(&config.cli.fail_on);
227-
let fail_on = Severity::parse(fail_on_str);
240+
let fail_on = if fail_on_str.eq_ignore_ascii_case("none") {
241+
None
242+
} else {
243+
match Severity::parse(fail_on_str) {
244+
Some(s) => Some(s),
245+
None => anyhow::bail!(
246+
"Unknown severity '{}' for --fail-on. Valid values: critical, major, minor, info, none",
247+
fail_on_str
248+
),
249+
}
250+
};
228251
if let Some(threshold) = fail_on
229252
&& all_findings.iter().any(|f| f.severity >= threshold)
230253
{

0 commit comments

Comments
 (0)