cratefield_core/
lint.rs

1//! The portable-SQL lint (ADR 0004): flags tokens that SQLite and Postgres
2//! disagree about, or that betray non-portable DDL. One canonical definition
3//! shared by `fz doctor` and the Postgres migration runner's set selection
4//! (issue #18) so the two can never drift.
5//!
6//! The lint reads **DDL**, not the whole file. Comments and string
7//! literals are stripped first, because neither is executed: a migration
8//! that explains itself in prose, or stores the word `blob` as data, is
9//! portable. Scanning them produced false positives that failed a module
10//! for a backtick inside a `--` comment.
11
12/// Returns `(token, explanation)` pairs found in `sql`.
13///
14/// This is the predicate behind both `fz doctor`'s migration lint and the
15/// `cratefield-adapter-postgres` runner's rule that a module's `sqlite`
16/// migration set may be applied to Postgres only when it passes.
17///
18/// SQL comments (`-- ...`, `/* ... */`) and single-quoted string literals
19/// are ignored: they are not DDL. Double-quoted identifiers are not
20/// ignored, because they are DDL and portable in both dialects.
21#[must_use]
22pub fn lint_portable_sql(sql: &str) -> Vec<(&'static str, &'static str)> {
23    const BANNED: &[(&str, &str)] = &[
24        (
25            "AUTOINCREMENT",
26            "SQLite-only; use plain INTEGER PRIMARY KEY (ULID ids instead of autoincrement)",
27        ),
28        (
29            "datetime(",
30            "dialect function; store ISO-8601 TEXT and compute in code",
31        ),
32        ("SERIAL", "Postgres-only; use TEXT ULID ids"),
33        (
34            "NOW()",
35            "dialect function; bind an ISO-8601 timestamp instead",
36        ),
37        (
38            "json_extract",
39            "SQLite-only JSON function; parse JSON in code",
40        ),
41        (
42            "`",
43            "backtick quoting is MySQL/SQLite; use double quotes or none",
44        ),
45        (
46            "BLOB",
47            "SQLite-only type; Postgres has BYTEA — ship a migrations/postgres override",
48        ),
49    ];
50
51    let haystack = strip_non_ddl(sql).to_ascii_lowercase();
52    BANNED
53        .iter()
54        .filter(|(token, _)| {
55            if *token == "`" {
56                haystack.contains('`')
57            } else {
58                haystack.contains(&token.to_ascii_lowercase())
59            }
60        })
61        .copied()
62        .collect()
63}
64
65/// Card-data fragments the harness never stores. With a normal Stripe
66/// integration the card details go straight to Stripe (Checkout, Elements, the
67/// SDKs) and never reach a backend, so a column or secret shaped like a card
68/// number, verification code or full expiry is a mistake. Deliberately the
69/// *specific* forms and not bare `pan`, `track` or `expiry`, which collide with
70/// legitimate columns — an audio `pan`, a music `track`, a `session_expiry`
71/// (issue #44).
72const CARD_DATA: &[&str] = &[
73    "card_number",
74    "cardnumber",
75    "card_no",
76    "cardno",
77    "primary_account_number",
78    "full_pan",
79    "cvv",
80    "cvc",
81    "cvv2",
82    "cvc2",
83    "card_cvv",
84    "card_cvc",
85    "exp_month",
86    "exp_year",
87    "card_expiry",
88    "expiry_date",
89    "expiration_date",
90    "track_data",
91    "magstripe",
92    "magnetic_stripe",
93];
94
95/// The first card-data fragment `text` contains (case-insensitive), or `None`.
96/// Keeps card numbers, verification codes and full expiry out of migrations and
97/// secret names — with a normal Stripe integration none of them should exist
98/// (issue #44).
99#[must_use]
100pub fn card_data_hit(text: &str) -> Option<&'static str> {
101    let hay = text.to_ascii_lowercase();
102    CARD_DATA.iter().copied().find(|frag| hay.contains(frag))
103}
104
105/// Card-data column or table names found in `sql`, as `(fragment, why)` pairs.
106/// Comments and string literals are ignored, so documenting the rule does not
107/// trip it.
108#[must_use]
109pub fn lint_card_data(sql: &str) -> Vec<(&'static str, &'static str)> {
110    match card_data_hit(&strip_non_ddl(sql)) {
111        Some(frag) => vec![(
112            frag,
113            "looks like card data; with a normal Stripe integration the card never \
114             reaches a backend — store only Stripe's identifiers",
115        )],
116        None => Vec::new(),
117    }
118}
119
120/// Replaces every SQL comment and single-quoted string literal with spaces,
121/// leaving the executable DDL and its byte positions alone.
122///
123/// Block comments nest in Postgres and do not in SQLite; this counts depth,
124/// which is right for Postgres and harmless for SQLite (a migration relying
125/// on the difference is not portable anyway). An unterminated comment or
126/// literal swallows the rest of the input rather than panicking — the
127/// database will reject it long before the lint matters.
128fn strip_non_ddl(sql: &str) -> String {
129    let bytes = sql.as_bytes();
130    let mut out = String::with_capacity(sql.len());
131    let mut i = 0;
132    while i < bytes.len() {
133        // `--` to end of line.
134        if bytes[i] == b'-' && bytes.get(i + 1) == Some(&b'-') {
135            while i < bytes.len() && bytes[i] != b'\n' {
136                out.push(' ');
137                i += 1;
138            }
139            continue;
140        }
141        // `/* ... */`, nesting.
142        if bytes[i] == b'/' && bytes.get(i + 1) == Some(&b'*') {
143            let mut depth = 1_usize;
144            out.push_str("  ");
145            i += 2;
146            while i < bytes.len() && depth > 0 {
147                if bytes[i] == b'/' && bytes.get(i + 1) == Some(&b'*') {
148                    depth += 1;
149                    out.push_str("  ");
150                    i += 2;
151                } else if bytes[i] == b'*' && bytes.get(i + 1) == Some(&b'/') {
152                    depth -= 1;
153                    out.push_str("  ");
154                    i += 2;
155                } else {
156                    out.push(if bytes[i] == b'\n' { '\n' } else { ' ' });
157                    i += 1;
158                }
159            }
160            continue;
161        }
162        // `'...'`, with `''` as the escape.
163        if bytes[i] == b'\'' {
164            out.push(' ');
165            i += 1;
166            while i < bytes.len() {
167                if bytes[i] == b'\'' {
168                    if bytes.get(i + 1) == Some(&b'\'') {
169                        out.push_str("  ");
170                        i += 2;
171                        continue;
172                    }
173                    out.push(' ');
174                    i += 1;
175                    break;
176                }
177                out.push(if bytes[i] == b'\n' { '\n' } else { ' ' });
178                i += 1;
179            }
180            continue;
181        }
182        // Not a byte we rewrite: copy the whole UTF-8 character.
183        let start = i;
184        i += 1;
185        while i < bytes.len() && (bytes[i] & 0xC0) == 0x80 {
186            i += 1;
187        }
188        out.push_str(&sql[start..i]);
189    }
190    out
191}
192
193#[cfg(test)]
194mod tests {
195    use super::lint_portable_sql;
196
197    #[test]
198    fn clean_portable_sql_passes() {
199        let sql = "CREATE TABLE t (id TEXT PRIMARY KEY, n INTEGER NOT NULL DEFAULT 0, \
200                   created_at TEXT NOT NULL, UNIQUE(id));";
201        assert!(lint_portable_sql(sql).is_empty());
202    }
203
204    #[test]
205    fn every_banned_token_is_flagged() {
206        let sql = "CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT, \
207                   at datetime('now'), s SERIAL, n NOW(), j json_extract(x,'$'), c `col`);";
208        let found = lint_portable_sql(sql);
209        let tokens: Vec<&str> = found.iter().map(|(token, _)| *token).collect();
210        assert_eq!(
211            tokens,
212            [
213                "AUTOINCREMENT",
214                "datetime(",
215                "SERIAL",
216                "NOW()",
217                "json_extract",
218                "`"
219            ]
220        );
221    }
222
223    #[test]
224    fn matching_is_case_insensitive() {
225        assert!(
226            lint_portable_sql("SELECT autoincrement FROM t;")
227                .iter()
228                .any(|(token, _)| *token == "AUTOINCREMENT")
229        );
230    }
231
232    #[test]
233    fn prose_in_a_comment_is_not_ddl() {
234        // The regression: `module-cms` explains its tables in a `--`
235        // comment, and markdown-style backticks around a table name failed
236        // the whole module for MySQL quoting it does not use.
237        let sql = "-- A small content store. `cms_item` is the working copy;\n\
238                   -- `cms_revision` is append-only history (stored as BLOB\n\
239                   -- in some other database, but not here).\n\
240                   CREATE TABLE cms_item (id TEXT PRIMARY KEY);";
241        assert_eq!(lint_portable_sql(sql), vec![]);
242    }
243
244    #[test]
245    fn a_block_comment_is_not_ddl_and_may_nest() {
246        let sql = "/* uses `backticks` and /* nests, mentioning SERIAL */ still inside */ \
247                   CREATE TABLE t (id TEXT PRIMARY KEY);";
248        assert_eq!(lint_portable_sql(sql), vec![]);
249    }
250
251    #[test]
252    fn a_string_literal_is_data_not_ddl() {
253        // Storing the word as a value says nothing about the column type.
254        let sql = "INSERT INTO kinds (name) VALUES ('blob'), ('serial'), \
255                   ('it''s NOW() in prose');";
256        assert_eq!(lint_portable_sql(sql), vec![]);
257    }
258
259    #[test]
260    fn stripping_comments_does_not_hide_real_ddl() {
261        // The other half: the same tokens outside a comment still fail, and
262        // a comment must not swallow the statement that follows it.
263        let sql = "-- a note about ids\n\
264                   CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT, c `col` TEXT);";
265        let tokens: Vec<&str> = lint_portable_sql(sql)
266            .iter()
267            .map(|(token, _)| *token)
268            .collect();
269        assert_eq!(tokens, ["AUTOINCREMENT", "`"]);
270    }
271
272    #[test]
273    fn a_double_quoted_identifier_is_still_ddl() {
274        // Double quotes are portable, so they are not stripped: a banned
275        // token inside them is a real column type, not prose.
276        let tokens: Vec<&str> = lint_portable_sql(r#"CREATE TABLE t ("data" BLOB);"#)
277            .iter()
278            .map(|(token, _)| *token)
279            .collect();
280        assert_eq!(tokens, ["BLOB"]);
281    }
282
283    #[test]
284    fn an_unterminated_comment_or_literal_does_not_panic() {
285        // Malformed SQL is the database's problem, not the lint's.
286        assert_eq!(lint_portable_sql("/* never closed"), vec![]);
287        assert_eq!(lint_portable_sql("SELECT 'never closed"), vec![]);
288        assert_eq!(lint_portable_sql("-- never newline"), vec![]);
289        // Multi-byte characters inside and outside the stripped spans.
290        assert_eq!(
291            lint_portable_sql("-- naïve ünicode ✓\nCREATE TABLE t (id TEXT);"),
292            vec![]
293        );
294    }
295
296    #[test]
297    fn now_without_parens_is_not_flagged() {
298        // `NOW()` is the dialect function; the word "now" in prose or a
299        // column name is not.
300        assert!(lint_portable_sql("SELECT now FROM t;").is_empty());
301    }
302}
303
304#[cfg(test)]
305mod card_data_tests {
306    use super::{card_data_hit, lint_card_data};
307
308    #[test]
309    fn a_card_number_column_is_flagged() {
310        let sql = "CREATE TABLE payment (id TEXT PRIMARY KEY, card_number TEXT)";
311        assert_eq!(lint_card_data(sql).len(), 1);
312        assert_eq!(lint_card_data(sql)[0].0, "card_number");
313        assert!(card_data_hit("cvv").is_some());
314        assert!(card_data_hit("exp_month").is_some());
315    }
316
317    #[test]
318    fn card_data_only_in_a_comment_or_string_is_not_flagged() {
319        // Documenting the rule must not trip it.
320        let sql = "CREATE TABLE t (id TEXT) -- never store card_number here\n";
321        assert!(lint_card_data(sql).is_empty());
322        let sql2 = "INSERT INTO note (body) VALUES ('do not store card_number')";
323        assert!(lint_card_data(sql2).is_empty());
324    }
325
326    #[test]
327    fn ambiguous_words_do_not_false_positive() {
328        // A music track, an audio pan, a session expiry are all legitimate.
329        for sql in [
330            "CREATE TABLE song (id TEXT, track INTEGER)",
331            "CREATE TABLE mix (id TEXT, pan REAL)",
332            "CREATE TABLE session (id TEXT, session_expiry TEXT)",
333            "CREATE TABLE t (id TEXT, token_expiry TEXT)",
334        ] {
335            assert!(lint_card_data(sql).is_empty(), "false positive on: {sql}");
336        }
337    }
338}
cratefield_core/lint.rs

cratefield_core/
lint.rs
