Skip to content

Commit b6274d7

Browse files
authored
linter: add ban-truncate-cascade rule (#453)
`truncate` with the `cascade` option set caused Linear's January 2024 incident: https://linear.app/blog/linear-incident-on-jan-24th-2024 related: #341
1 parent 115b33b commit b6274d7

File tree

12 files changed

+374
-126
lines changed

12 files changed

+374
-126
lines changed

crates/squawk_linter/src/lib.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use rules::ban_drop_column;
3434
use rules::ban_drop_database;
3535
use rules::ban_drop_not_null;
3636
use rules::ban_drop_table;
37+
use rules::ban_truncate_cascade;
3738
use rules::changing_column_type;
3839
use rules::constraint_missing_not_valid;
3940
use rules::disallow_unique_constraint;
@@ -108,6 +109,8 @@ pub enum Rule {
108109
BanCreateDomainWithConstraint,
109110
#[serde(rename = "ban-alter-domain-with-add-constraint")]
110111
BanAlterDomainWithAddConstraint,
112+
#[serde(rename = "ban-truncate-cascade")]
113+
BanTruncateCascade,
111114
// xtask:new-rule:error-name
112115
}
113116

@@ -145,6 +148,7 @@ impl TryFrom<&str> for Rule {
145148
}
146149
"ban-create-domain-with-constraint" => Ok(Rule::BanCreateDomainWithConstraint),
147150
"ban-alter-domain-with-add-constraint" => Ok(Rule::BanAlterDomainWithAddConstraint),
151+
"ban-truncate-cascade" => Ok(Rule::BanTruncateCascade),
148152
// xtask:new-rule:str-name
149153
_ => Err(format!("Unknown violation name: {}", s)),
150154
}
@@ -202,6 +206,7 @@ impl fmt::Display for Rule {
202206
Rule::BanCreateDomainWithConstraint => "ban-create-domain-with-constraint",
203207
Rule::UnusedIgnore => "unused-ignore",
204208
Rule::BanAlterDomainWithAddConstraint => "ban-alter-domain-with-add-constraint",
209+
Rule::BanTruncateCascade => "ban-truncate-cascade",
205210
// xtask:new-rule:variant-to-name
206211
};
207212
write!(f, "{}", val)
@@ -345,6 +350,9 @@ impl Linter {
345350
if self.rules.contains(&Rule::TransactionNesting) {
346351
transaction_nesting(self, &file);
347352
}
353+
if self.rules.contains(&Rule::BanTruncateCascade) {
354+
ban_truncate_cascade(self, &file);
355+
}
348356
// xtask:new-rule:rule-call
349357

350358
// locate any ignores in the file
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
use squawk_syntax::{
2+
ast::{self, HasModuleItem},
3+
Parse, SourceFile,
4+
};
5+
6+
use crate::{Linter, Rule, Violation};
7+
8+
pub(crate) fn ban_truncate_cascade(ctx: &mut Linter, parse: &Parse<SourceFile>) {
9+
let file = parse.tree();
10+
for item in file.items() {
11+
match item {
12+
ast::Item::Truncate(truncate) => {
13+
if let Some(cascade) = truncate.cascade_token() {
14+
// TODO: if we had knowledge about the entire schema, we
15+
// could be more precise here and actually navigate the
16+
// foreign keys.
17+
ctx.report(Violation::new(
18+
Rule::BanTruncateCascade,
19+
format!("Using `CASCADE` will recursively truncate any tables that foreign key to the referenced tables! So if you had foreign keys setup as `a <- b <- c` and truncated `a`, then `b` & `c` would also be truncated!"),
20+
cascade.text_range(),
21+
"Remove the `CASCADE` and specify exactly which tables you want to truncate.".to_string(),
22+
));
23+
}
24+
}
25+
_ => (),
26+
}
27+
}
28+
}
29+
30+
#[cfg(test)]
31+
mod test {
32+
use insta::assert_debug_snapshot;
33+
34+
use crate::{Linter, Rule};
35+
use squawk_syntax::SourceFile;
36+
37+
#[test]
38+
fn err() {
39+
let sql = r#"
40+
truncate a, b, c cascade;
41+
"#;
42+
let file = SourceFile::parse(sql);
43+
let mut linter = Linter::from([Rule::BanTruncateCascade]);
44+
let errors = linter.lint(file, sql);
45+
assert_ne!(errors.len(), 0);
46+
assert_debug_snapshot!(errors);
47+
}
48+
49+
#[test]
50+
fn ok() {
51+
let sql = r#"
52+
truncate a, b, c;
53+
truncate a;
54+
"#;
55+
let file = SourceFile::parse(sql);
56+
let mut linter = Linter::from([Rule::BanTruncateCascade]);
57+
let errors = linter.lint(file, sql);
58+
assert_eq!(errors.len(), 0);
59+
}
60+
}

crates/squawk_linter/src/rules/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ pub(crate) mod ban_drop_column;
1111
pub(crate) mod ban_drop_database;
1212
pub(crate) mod ban_drop_not_null;
1313
pub(crate) mod ban_drop_table;
14+
pub(crate) mod ban_truncate_cascade;
1415
pub(crate) mod changing_column_type;
1516
pub(crate) mod constraint_missing_not_valid;
1617
pub(crate) mod disallow_unique_constraint;
@@ -40,6 +41,7 @@ pub(crate) use ban_drop_column::ban_drop_column;
4041
pub(crate) use ban_drop_database::ban_drop_database;
4142
pub(crate) use ban_drop_not_null::ban_drop_not_null;
4243
pub(crate) use ban_drop_table::ban_drop_table;
44+
pub(crate) use ban_truncate_cascade::ban_truncate_cascade;
4345
pub(crate) use changing_column_type::changing_column_type;
4446
pub(crate) use constraint_missing_not_valid::constraint_missing_not_valid;
4547
pub(crate) use disallow_unique_constraint::disallow_unique_constraint;
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
source: crates/squawk_linter/src/rules/ban_truncate_cascade.rs
3+
expression: errors
4+
---
5+
[
6+
Violation {
7+
code: BanTruncateCascade,
8+
message: "Using `CASCADE` will recursively truncate any tables that foreign key to the referenced tables! So if you had foreign keys setup as `a <- b <- c` and truncated `a`, then `b` & `c` would also be truncated!",
9+
text_range: 26..33,
10+
help: Some(
11+
"Remove the `CASCADE` and specify exactly which tables you want to truncate.",
12+
),
13+
},
14+
]

crates/squawk_parser/src/grammar.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9781,11 +9781,7 @@ fn lock_stmt(p: &mut Parser<'_>) -> CompletedMarker {
97819781
p.bump(LOCK_KW);
97829782
// [ TABLE ]
97839783
p.eat(TABLE_KW);
9784-
// [ ONLY ] name [ * ] [, ...]
9785-
relation_name(p);
9786-
while !p.at(EOF) && p.eat(COMMA) {
9787-
relation_name(p);
9788-
}
9784+
table_list(p);
97899785
// [ IN lockmode MODE ]
97909786
if p.eat(IN_KW) {
97919787
match (p.current(), p.nth(1)) {
@@ -9828,6 +9824,16 @@ fn lock_stmt(p: &mut Parser<'_>) -> CompletedMarker {
98289824
m.complete(p, LOCK_STMT)
98299825
}
98309826

9827+
// [ ONLY ] name [ * ] [, ... ]
9828+
fn table_list(p: &mut Parser<'_>) {
9829+
let m = p.start();
9830+
relation_name(p);
9831+
while !p.at(EOF) && p.eat(COMMA) {
9832+
relation_name(p);
9833+
}
9834+
m.complete(p, TABLE_LIST);
9835+
}
9836+
98319837
// [ WITH with_query [, ...] ]
98329838
// MERGE INTO [ ONLY ] target_table_name [ * ] [ [ AS ] target_alias ]
98339839
// USING data_source ON join_condition
@@ -11098,12 +11104,7 @@ fn truncate_stmt(p: &mut Parser<'_>) -> CompletedMarker {
1109811104
let m = p.start();
1109911105
p.bump(TRUNCATE_KW);
1110011106
p.eat(TABLE_KW);
11101-
while !p.at(EOF) {
11102-
relation_name(p);
11103-
if !p.eat(COMMA) {
11104-
break;
11105-
}
11106-
}
11107+
table_list(p);
1110711108
if p.eat(RESTART_KW) {
1110811109
p.expect(IDENTITY_KW);
1110911110
}

0 commit comments

Comments
 (0)