Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions specs/schema/schema.spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ Parses SQL schema files (migrations) and spec markdown to build table/column map
| Function | Parameters | Returns | Description |
|----------|-----------|---------|-------------|
| `build_schema` | `schema_dir: &Path` | `HashMap<String, SchemaTable>` | Build a complete schema map from SQL/migration files in the given directory, sorted by filename |
| `schema_read_errors` | `schema_dir: &Path` | `Vec<String>` | Error message for each schema/migration file that exists but cannot be read as UTF-8, so the validation gate can fail loud instead of silently under-validating (`build_schema` skips such files) |
| `parse_spec_schema` | `body: &str` | `HashMap<String, Vec<SpecColumn>>` | Extract column definitions from a spec's `### Schema` section(s) |

## Invariants

1. `build_schema` replays migrations in filename-sorted order for deterministic results
2. `build_schema` returns an empty map if the directory does not exist
2a. A schema/migration file that exists but cannot be read as UTF-8 is silently skipped by `build_schema` (its tables/columns vanish); `schema_read_errors` reports each such file so the validation gate surfaces it as a hard error rather than letting an all-unreadable schema silently disable `db_tables`/column checks
3. Column types are normalized to uppercase (e.g. "integer" becomes "INTEGER")
4. ALTER TABLE ADD COLUMN is idempotent — duplicate column names are skipped
5. DROP TABLE removes the table and all its columns from the map
Expand Down
16 changes: 16 additions & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,22 @@ pub fn run_validation(
}
}

// Schema/migration files that exist but can't be read as UTF-8 are silently
// skipped by build_schema — their tables/columns vanish, which can silently
// disable db_tables/column validation (an all-unreadable schema makes the
// checks a no-op). Surface them once, project-level (not per spec), as hard
// errors so the gate fails loud instead of under-validating.
if let Some(dir) = &config.schema_dir {
for err in schema::schema_read_errors(&root.join(dir)) {
total_errors += 1;
if collect {
all_errors.push(err);
} else {
println!("\n{} {err}", "✗".red());
}
}
}

if !collect && drafts_skipped > 0 {
println!(
"\n{} {drafts_skipped} draft spec(s) skipped section and export validation — set `status: active` to enable full checks",
Expand Down
73 changes: 73 additions & 0 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,53 @@ pub fn build_schema(schema_dir: &Path) -> HashMap<String, SchemaTable> {
tables
}

/// Return an error message for each schema/migration file that EXISTS in
/// `schema_dir` but could not be read as UTF-8. `build_schema` silently skips such
/// a file (`Err(_) => continue`), which makes its tables/columns vanish and can
/// silently disable DB validation — if every schema file is unreadable, the
/// discovered set is empty and the `db_tables`/column checks become a no-op with
/// no signal. The validation gate surfaces these so an unreadable migration fails
/// loud instead of quietly weakening the guarantee. Scans the same files
/// `build_schema` would (matching `SQL_EXTENSIONS`); returns an empty vec when the
/// directory is absent (schema is simply not configured for this project).
pub fn schema_read_errors(schema_dir: &Path) -> Vec<String> {
let mut errors = Vec::new();
if !schema_dir.exists() {
return errors;
}

let mut files: Vec<_> = fs::read_dir(schema_dir)
.into_iter()
.flatten()
.flatten()
.filter(|e| {
let ext = e
.path()
.extension()
.and_then(|x| x.to_str())
.unwrap_or("")
.to_string();
SQL_EXTENSIONS.contains(&ext.as_str())
})
.collect();
Comment thread
0xLeif marked this conversation as resolved.
Outdated
files.sort_by_key(|e| e.file_name());

for entry in &files {
let path = entry.path();
if path.is_file() && fs::read_to_string(&path).is_err() {
let name = path
.file_name()
.and_then(|n| n.to_str())
.unwrap_or("<unknown>");
errors.push(format!(
"Schema/migration file `{name}` could not be read as UTF-8; DB schema validation would be incomplete"
));
}
}

errors
}

/// Parse SQL content and merge discovered tables/columns into the map.
fn parse_sql_into(sql: &str, tables: &mut HashMap<String, SchemaTable>) {
// Handle CREATE TABLE statements
Expand Down Expand Up @@ -694,6 +741,32 @@ Something
assert!(tables.is_empty());
}

#[test]
fn test_schema_read_errors_flags_only_unreadable() {
use std::io::Write;
let tmp = tempfile::tempdir().unwrap();
let dir = tmp.path();

// Absent dir → no errors (schema simply not configured).
assert!(schema_read_errors(Path::new("/nonexistent/path")).is_empty());

// A readable migration → no error.
fs::write(dir.join("001_ok.sql"), "CREATE TABLE t (id INTEGER);").unwrap();
assert!(schema_read_errors(dir).is_empty());

// A non-source file present alongside → ignored (not a SQL_EXTENSIONS file).
fs::write(dir.join("README.md"), "notes").unwrap();
assert!(schema_read_errors(dir).is_empty());

// A non-UTF-8 migration → exactly one error naming the file.
let mut f = std::fs::File::create(dir.join("002_bad.sql")).unwrap();
f.write_all(b"CREATE TABLE u (id INTEGER);\n\xff\xfe")
.unwrap();
let errs = schema_read_errors(dir);
assert_eq!(errs.len(), 1, "only the unreadable file should be flagged");
assert!(errs[0].contains("002_bad.sql"));
}

#[test]
fn test_build_schema_migration_ordering() {
let tmp = tempfile::tempdir().unwrap();
Expand Down
70 changes: 70 additions & 0 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5959,6 +5959,76 @@ None
.stdout(predicate::str::contains("nothing to validate").not());
}

#[test]
fn check_gates_on_unreadable_schema_file() {
// Regression: an unreadable (non-UTF-8) migration was silently skipped by
// build_schema, so its tables vanished and db_tables validation became a
// no-op — check passed green even under strict. It must now be a hard error.
let tmp = TempDir::new().unwrap();
let root = tmp.path();
// enforcement: strict so a validation error gates the exit code without the
// --strict flag (mirrors the schema-drift test above).
fs::write(
root.join(".specsync.toml"),
"specs_dir = \"specs\"\nsource_dirs = [\"src\"]\nschema_dir = \"db/migrations\"\nenforcement = \"strict\"\n",
)
.unwrap();
fs::create_dir_all(root.join("src")).unwrap();
fs::write(root.join("src/a.rs"), "pub fn f() {}\n").unwrap();
fs::create_dir_all(root.join("db/migrations")).unwrap();
let mut bad = b"CREATE TABLE users (id INTEGER);\n".to_vec();
bad.push(0xFF);
fs::write(root.join("db/migrations/001_init.sql"), bad).unwrap();
fs::create_dir_all(root.join("specs/m")).unwrap();
fs::write(
root.join("specs/m/m.spec.md"),
"---\nmodule: m\nversion: 1\nstatus: active\nfiles:\n - src/a.rs\ndb_tables:\n - users\n---\n# m\n## Purpose\np\n",
)
.unwrap();

// The unreadable migration is surfaced as a hard error AND gates the exit.
specsync()
.current_dir(root)
.arg("check")
.assert()
.failure()
.stdout(predicate::str::contains(
"could not be read as UTF-8; DB schema",
));
}

#[test]
fn check_no_schema_error_for_readable_migration() {
// No false positive: a readable migration produces no schema-read error.
let tmp = TempDir::new().unwrap();
let root = tmp.path();
fs::write(
root.join(".specsync.toml"),
"specs_dir = \"specs\"\nsource_dirs = [\"src\"]\nschema_dir = \"db/migrations\"\n",
)
.unwrap();
fs::create_dir_all(root.join("src")).unwrap();
fs::write(root.join("src/a.rs"), "pub fn f() {}\n").unwrap();
fs::create_dir_all(root.join("db/migrations")).unwrap();
fs::write(
root.join("db/migrations/001_init.sql"),
"CREATE TABLE users (id INTEGER);\n",
)
.unwrap();
fs::create_dir_all(root.join("specs/m")).unwrap();
fs::write(
root.join("specs/m/m.spec.md"),
"---\nmodule: m\nversion: 1\nstatus: active\nfiles:\n - src/a.rs\ndb_tables:\n - users\n---\n# m\n## Purpose\np\n",
)
.unwrap();

specsync()
.current_dir(root)
.arg("check")
.assert()
.stdout(predicate::str::contains("could not be read as UTF-8; DB schema").not());
}

// ─── specsync merge ─────────────────────────────────────────────────────

/// Regression (CRITICAL): `merge` must never write a corrupt spec. A conflict
Expand Down
Loading