diff --git a/BUILDING.md b/BUILDING.md index 4b42973..c2bea8d 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -47,7 +47,7 @@ docker run -d \ -e SONAR_ES_BOOTSTRAP_CHECKS_DISABLE=true \ -p 9000:9000 \ -v $(pwd)/target/sonar-postgres-plugin-1.4-SNAPSHOT.jar:/opt/sonarqube/extensions/plugins/sonar-postgres-plugin-1.4-SNAPSHOT.jar \ - sonarqube:lts-community + sonarqube:24.12.0.100206-community xdg-open http://localhost:9000/ docker logs -f sonarqube ``` diff --git a/README.md b/README.md index 6304b9e..09b4665 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ Check [BUILDING.md](BUILDING.md) * [ban-drop-database](src/main/resources/com/premiumminds/sonar/postgres/ban-drop-database.md) * [ban-alter-domain-with-add-constraint](src/main/resources/com/premiumminds/sonar/postgres/ban-alter-domain-with-add-constraint.md) * [ban-create-domain-with-constraint](src/main/resources/com/premiumminds/sonar/postgres/ban-create-domain-with-constraint.md) + * [ban-truncate-cascade](src/main/resources/com/premiumminds/sonar/postgres/ban-truncate-cascade.md) * [changing-column-type](src/main/resources/com/premiumminds/sonar/postgres/changing-column-type.md) * [cluster](src/main/resources/com/premiumminds/sonar/postgres/cluster.md) * [concurrently](src/main/resources/com/premiumminds/sonar/postgres/concurrently.md) diff --git a/samples/ban-truncate-cascade/file1.sql b/samples/ban-truncate-cascade/file1.sql new file mode 100644 index 0000000..bb0348e --- /dev/null +++ b/samples/ban-truncate-cascade/file1.sql @@ -0,0 +1 @@ +TRUNCATE a, b, c CASCADE; diff --git a/samples/ban-truncate-cascade/file2-ok.sql b/samples/ban-truncate-cascade/file2-ok.sql new file mode 100644 index 0000000..e46c591 --- /dev/null +++ b/samples/ban-truncate-cascade/file2-ok.sql @@ -0,0 +1 @@ +TRUNCATE a, b, c; diff --git a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java index de77c16..71f8c0a 100644 --- a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java +++ b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlQualityProfile.java @@ -10,6 +10,7 @@ import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_CHAR_FIELD; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_CREATE_DOMAIN_WITH_CONSTRAINT; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_DROP_DATABASE; +import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_TRUNCATE_CASCADE; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_CHANGING_COLUMN_TYPE; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_CLUSTER; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_CONCURRENTLY; @@ -45,6 +46,7 @@ public void define(Context context) { activateRule(profile, RULE_ADDING_SERIAL_PRIMARY_KEY_FIELD); activateRule(profile, RULE_BAN_CHAR_FIELD); activateRule(profile, RULE_BAN_ALTER_DOMAIN_WITH_CONSTRAINT); + activateRule(profile, RULE_BAN_TRUNCATE_CASCADE); activateRule(profile, RULE_BAN_CREATE_DOMAIN_WITH_CONSTRAINT); activateRule(profile, RULE_BAN_DROP_DATABASE); activateRule(profile, RULE_CHANGING_COLUMN_TYPE); diff --git a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java index 3eec065..8287e0d 100644 --- a/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java +++ b/src/main/java/com/premiumminds/sonar/postgres/PostgresSqlRulesDefinition.java @@ -10,6 +10,7 @@ import com.premiumminds.sonar.postgres.visitors.BanCharFieldVisitorCheck; import com.premiumminds.sonar.postgres.visitors.BanCreateDomainWithConstraintCheck; import com.premiumminds.sonar.postgres.visitors.BanDropDatabaseVisitorCheck; +import com.premiumminds.sonar.postgres.visitors.BanTruncateCascade; import com.premiumminds.sonar.postgres.visitors.ChangingColumnTypeVisitorCheck; import com.premiumminds.sonar.postgres.visitors.ClusterVisitorCheck; import com.premiumminds.sonar.postgres.visitors.ConcurrentVisitorCheck; @@ -46,6 +47,7 @@ public class PostgresSqlRulesDefinition implements RulesDefinition { public static final RuleKey RULE_BAN_CHAR_FIELD = RuleKey.of(REPOSITORY, "ban-char-field"); public static final RuleKey RULE_BAN_CREATE_DOMAIN_WITH_CONSTRAINT = RuleKey.of(REPOSITORY, "ban-create-domain-with-constraint"); public static final RuleKey RULE_BAN_ALTER_DOMAIN_WITH_CONSTRAINT = RuleKey.of(REPOSITORY, "ban-alter-domain-with-add-constraint"); + public static final RuleKey RULE_BAN_TRUNCATE_CASCADE = RuleKey.of(REPOSITORY, "ban-truncate-cascade"); public static final RuleKey RULE_BAN_DROP_DATABASE = RuleKey.of(REPOSITORY, "ban-drop-database"); public static final RuleKey RULE_CHANGING_COLUMN_TYPE = RuleKey.of(REPOSITORY, "changing-column-type"); public static final RuleKey RULE_CONSTRAINT_MISSING_NOT_VALID = RuleKey.of(REPOSITORY, "constraint-missing-not-valid"); @@ -119,6 +121,11 @@ public void define(Context context) { .setType(RuleType.BUG) .setMarkdownDescription(getClass().getResource("ban-alter-domain-with-add-constraint.md")); + repository.createRule(RULE_BAN_TRUNCATE_CASCADE.rule()) + .setName("ban-truncate-cascade") + .setType(RuleType.BUG) + .setMarkdownDescription(getClass().getResource("ban-truncate-cascade.md")); + repository.createRule(RULE_BAN_DROP_DATABASE.rule()) .setName("ban-drop-database rule") .setType(RuleType.BUG) @@ -220,6 +227,7 @@ public static List allChecks(){ new BanCharFieldVisitorCheck(), new BanCreateDomainWithConstraintCheck(), new BanAlterDomainWithConstraintCheck(), + new BanTruncateCascade(), new PreferTextFieldVisitorCheck(), new VacuumFullVisitorCheck(), new ClusterVisitorCheck(), diff --git a/src/main/java/com/premiumminds/sonar/postgres/visitors/BanTruncateCascade.java b/src/main/java/com/premiumminds/sonar/postgres/visitors/BanTruncateCascade.java new file mode 100644 index 0000000..ee73e77 --- /dev/null +++ b/src/main/java/com/premiumminds/sonar/postgres/visitors/BanTruncateCascade.java @@ -0,0 +1,27 @@ +package com.premiumminds.sonar.postgres.visitors; + +import com.premiumminds.sonar.postgres.protobuf.DropBehavior; +import com.premiumminds.sonar.postgres.protobuf.TruncateStmt; +import org.sonar.api.rule.RuleKey; +import org.sonar.check.Rule; + +import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_TRUNCATE_CASCADE; + +@Rule(key = "ban-truncate-cascade") +public class BanTruncateCascade extends AbstractVisitorCheck { + + @Override + public void visit(TruncateStmt truncateStmt) { + + if (truncateStmt.getBehavior() == DropBehavior.DROP_CASCADE){ + newIssue("Truncate cascade will recursively truncate all related tables"); + } + + super.visit(truncateStmt); + } + + @Override + protected RuleKey getRule() { + return RULE_BAN_TRUNCATE_CASCADE; + } +} diff --git a/src/main/java/com/premiumminds/sonar/postgres/visitors/OnlySchemaMigrationsVisitorCheck.java b/src/main/java/com/premiumminds/sonar/postgres/visitors/OnlySchemaMigrationsVisitorCheck.java index 43b21f9..0a263f9 100644 --- a/src/main/java/com/premiumminds/sonar/postgres/visitors/OnlySchemaMigrationsVisitorCheck.java +++ b/src/main/java/com/premiumminds/sonar/postgres/visitors/OnlySchemaMigrationsVisitorCheck.java @@ -1,16 +1,14 @@ package com.premiumminds.sonar.postgres.visitors; -import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_ONLY_SCHEMA_MIGRATIONS; - import com.premiumminds.sonar.postgres.protobuf.DeleteStmt; -import com.premiumminds.sonar.postgres.protobuf.DoStmt; -import com.premiumminds.sonar.postgres.protobuf.DropStmt; import com.premiumminds.sonar.postgres.protobuf.InsertStmt; import com.premiumminds.sonar.postgres.protobuf.TruncateStmt; import com.premiumminds.sonar.postgres.protobuf.UpdateStmt; import org.sonar.api.rule.RuleKey; import org.sonar.check.Rule; +import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_ONLY_SCHEMA_MIGRATIONS; + @Rule(key = "only-schema-migrations") public class OnlySchemaMigrationsVisitorCheck extends AbstractVisitorCheck { diff --git a/src/main/resources/com/premiumminds/sonar/postgres/ban-truncate-cascade.md b/src/main/resources/com/premiumminds/sonar/postgres/ban-truncate-cascade.md new file mode 100644 index 0000000..6ca76fa --- /dev/null +++ b/src/main/resources/com/premiumminds/sonar/postgres/ban-truncate-cascade.md @@ -0,0 +1,72 @@ +== problem + +Using `TRUNCATE`'s `CASCADE` option will truncate any tables that are also foreign-keyed to the specified tables. + +So if you had tables with foreign-keys like: + +`` +a <- b <- c +`` + +and ran: + +``sql +truncate a cascade; +`` + +You'd end up with `a`, `b`, & `c` all being truncated! + +=== runnable example + +Setup: + +``sql +create table a ( + a_id int primary key +); +create table b ( + b_id int primary key, + a_id int, + foreign key (a_id) references a(a_id) +); +create table c ( + c_id int primary key, + b_id int, + foreign key (b_id) references b(b_id) +); +insert into a (a_id) values (1), (2), (3); +insert into b (b_id, a_id) values (101, 1), (102, 2), (103, 3); +insert into c (c_id, b_id) values (1001, 101), (1002, 102), (1003, 103); +`` + +Then you run: + +``sql +truncate a cascade; +`` + +Which outputs: + +``text +NOTICE: truncate cascades to table "b" +NOTICE: truncate cascades to table "c" + +Query 1 OK: TRUNCATE TABLE +`` + +And now tables `a`, `b`, & `c` are empty! + +== solution + +Don't use the `CASCADE` option, instead manually specify the tables you want. + +So if you just wanted tables `a` and `b` from the example above: + +``sql +truncate a, b; +`` + +== links + + * https://www.postgresql.org/docs/current/sql-truncate.html + * `CASCADE`'s recursive nature [caused Linear's 2024-01-24 incident](https://linear.app/blog/linear-incident-on-jan-24th-2024). diff --git a/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java b/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java index b2f0095..29a4d43 100644 --- a/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java +++ b/src/test/java/com/premiumminds/sonar/postgres/PostgresSqlSensorTest.java @@ -27,6 +27,7 @@ import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_CHAR_FIELD; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_CREATE_DOMAIN_WITH_CONSTRAINT; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_DROP_DATABASE; +import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_BAN_TRUNCATE_CASCADE; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_CHANGING_COLUMN_TYPE; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_CLUSTER; import static com.premiumminds.sonar.postgres.PostgresSqlRulesDefinition.RULE_CONCURRENTLY; @@ -97,6 +98,7 @@ void testAllRules() { RULE_BAN_DROP_DATABASE, RULE_BAN_CREATE_DOMAIN_WITH_CONSTRAINT, RULE_BAN_ALTER_DOMAIN_WITH_CONSTRAINT, + RULE_BAN_TRUNCATE_CASCADE, RULE_CHANGING_COLUMN_TYPE, RULE_CONSTRAINT_MISSING_NOT_VALID, RULE_DISALLOWED_UNIQUE_CONSTRAINT, @@ -939,6 +941,25 @@ void banAlterDomainWithConstraints() { assertEquals(1, fileMap.size()); } + @Test + void banTruncateCascade() { + createFile(contextTester, "file1.sql", "TRUNCATE a, b, c CASCADE;\n"); + createFile(contextTester, "file2-ok.sql", "TRUNCATE a, b, c;"); + + final RuleKey rule = RULE_BAN_TRUNCATE_CASCADE; + PostgresSqlSensor sensor = getPostgresSqlSensor(rule); + sensor.execute(contextTester); + + final Map> issueMap = groupByRuleAndFile(contextTester.allIssues()); + + final Map fileMap = issueMap.get(rule); + + assertEquals("Truncate cascade will recursively truncate all related tables", + fileMap.get(":file1.sql").primaryLocation().message()); + + assertEquals(1, fileMap.size()); + } + private PostgresSqlSensor getPostgresSqlSensor(RuleKey... ruleKey) { ActiveRulesBuilder activeRulesBuilder = new ActiveRulesBuilder();