Fix: schema-discovery fail-open (unreadable migration silently disabled DB validation)

specs/schema/schema.spec.md

@@ -1,6 +1,6 @@
 ---
 module: schema
-version: 1
+version: 2
 status: stable
 files:
   - src/schema.rs
@@ -36,12 +36,15 @@ 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, plus one for a `schema_dir` that exists but cannot be enumerated (unreadable, or a file rather than a directory), so the validation gate can fail loud instead of silently under-validating (`build_schema` skips such files / returns empty) |
 | `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
+2b. A `schema_dir` that exists but cannot be enumerated by `read_dir` — because it is unreadable or is a file rather than a directory — is itself a hard error from `schema_read_errors` (not a silent empty result): the same fail-open would otherwise leave schema discovery empty and skip `db_tables`/column checks with no signal
 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
@@ -97,8 +100,9 @@ Parses SQL schema files (migrations) and spec markdown to build table/column map
 
 | Condition | Behavior |
 |-----------|----------|
-| Schema directory does not exist | `build_schema` returns empty map |
-| File cannot be read | File is silently skipped |
+| Schema directory does not exist | `build_schema` returns empty map; `schema_read_errors` returns no errors (schema simply not configured) |
+| Schema directory exists but cannot be enumerated (unreadable, or a file not a directory) | `schema_read_errors` returns a hard error naming the directory (fail-loud); `build_schema` returns an empty map |
+| File cannot be read | `build_schema` silently skips the file; `schema_read_errors` flags it as a hard error |
 | Unmatched parentheses in CREATE TABLE | `extract_paren_body` returns `None`, table is skipped |
 | No `### Schema` section in spec | `parse_spec_schema` returns empty map |
 | Column name looks like SQL keyword | Column is skipped by `is_sql_keyword` check |
@@ -123,4 +127,5 @@ Parses SQL schema files (migrations) and spec markdown to build table/column map
 
 | Date | Change |
 |------|--------|
+| 2026-07-06 | `schema_read_errors` now also fails loud when `schema_dir` exists but cannot be enumerated by `read_dir` (unreadable, or a file not a directory) — closing the same fail-open; added invariant 2b, updated the API row and Error Cases table |
 | 2026-03-29 | Initial spec |

src/commands/mod.rs

@@ -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",

src/schema.rs

@@ -103,6 +103,65 @@ 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;
+    }
+
+    // A `schema_dir` that exists but cannot be enumerated (unreadable, or a file
+    // rather than a directory) makes `read_dir` return `Err`. Ignoring it would be
+    // the same fail-open this function exists to close: schema discovery would come
+    // back empty and the `db_tables`/column checks would be silently skipped. Surface
+    // it as a hard error instead.
+    let read_dir = match fs::read_dir(schema_dir) {
+        Ok(read_dir) => read_dir,
+        Err(err) => {
+            errors.push(format!(
+                "Schema directory `{}` could not be read ({err}); DB schema validation would be incomplete",
+                schema_dir.display()
+            ));
+            return errors;
+        }
+    };
+
+    let mut files: Vec<_> = read_dir
+        .flatten()
+        .filter(|e| {
+            e.path()
+                .extension()
+                .and_then(|ext| ext.to_str())
+                .map(|ext| SQL_EXTENSIONS.contains(&ext))
+                .unwrap_or(false)
+        })
+        .collect();
+    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
@@ -694,6 +753,46 @@ 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_schema_read_errors_flags_unenumerable_dir() {
+        // A `schema_dir` that exists but is a file rather than a directory makes
+        // `read_dir` return `Err`. That must fail loud (hard error), not fail open
+        // to an empty schema that would silently skip db_tables/column checks.
+        let tmp = tempfile::tempdir().unwrap();
+        let not_a_dir = tmp.path().join("schema.sql");
+        fs::write(&not_a_dir, "CREATE TABLE t (id INTEGER);").unwrap();
+
+        let errs = schema_read_errors(&not_a_dir);
+        assert_eq!(errs.len(), 1, "an unenumerable schema dir must be flagged");
+        assert!(errs[0].contains("could not be read"));
+    }
+
     #[test]
     fn test_build_schema_migration_ordering() {
         let tmp = tempfile::tempdir().unwrap();

tests/integration.rs

@@ -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