From e6ab072d057566ed902cf4215e3a0afb5553af14 Mon Sep 17 00:00:00 2001 From: dsecurity49 Date: Sat, 5 Sep 2026 16:27:56 +0530 Subject: [PATCH 1/2] Upgrade Squawk and guard GitHub environment setup --- CHANGELOG.md | 10 +++ Cargo.lock | 18 ++--- Cargo.toml | 10 +-- docs/CONTRACT.md | 2 +- docs/GITHUB_ACTIONS.md | 5 +- src/_internal/ast/visitor.rs | 95 ++++++++++++++++--------- src/_internal/ast/visitor_tests.rs | 110 +++++++++++++++++++++++++++-- src/cli_init.rs | 93 ++++++++++++++++++++++++ 8 files changed, 289 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c85ccb..3fba323 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ commits and pull requests. Published binaries, checksums, and generated release notes are available on the [GitHub Releases page](https://github.com/dsecurity49/safe-migrate/releases). +## v0.8.1 — Unreleased + +- Upgraded Squawk's parser, lexer, syntax tree, and linter to 2.64.0, including + its stricter routine validation and more precise type and transaction nodes. +- Preserved ordered-set aggregate and function signatures across the new AST, + and stopped prepared-transaction completion from ending the analyzed + migration transaction. +- Warn before GitHub secret setup when the baseline environment is missing, + unverifiable, or lacks reviewer or deployment-branch protection. + ## v0.8.0 — 2026-09-04 - Rule evaluation now uses one immutable context with explicit capability diff --git a/Cargo.lock b/Cargo.lock index 5b2efd6..7c22e07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1020,7 +1020,7 @@ checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" [[package]] name = "safe-migrate" -version = "0.8.0" +version = "0.8.1" dependencies = [ "anyhow", "assert_cmd", @@ -1200,9 +1200,9 @@ dependencies = [ [[package]] name = "squawk-lexer" -version = "2.63.0" +version = "2.64.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "efaba81a4ba786561a9636eacab44bdd90b3ed35ab547fddab9c912ea52afbb1" +checksum = "09450f444bc7734433a9167d65c9d05a99135e65eb1093ac0e5e3c797c63415b" [[package]] name = "squawk-line-index" @@ -1217,9 +1217,9 @@ dependencies = [ [[package]] name = "squawk-linter" -version = "2.63.0" +version = "2.64.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2f01fc98f8bbe117e591f23a276af0242dec79585a3060ca5df393e922fa6f6" +checksum = "5805ee8b3750bd815b8ebba49b2e2a65416f29d89cd38503703b350132055f7b" dependencies = [ "annotate-snippets", "enum-iterator", @@ -1233,9 +1233,9 @@ dependencies = [ [[package]] name = "squawk-parser" -version = "2.63.0" +version = "2.64.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "58327f2c81cd96cfc80c4af6f6d533ca0fe38c38f3c323be0eeb1fcc3677bf28" +checksum = "14ad5fa5581a0f76a06d7f7702ee8b6a15e26997d1e0b64c02218c7e00b5778b" dependencies = [ "drop_bomb", "squawk-lexer", @@ -1243,9 +1243,9 @@ dependencies = [ [[package]] name = "squawk-syntax" -version = "2.63.0" +version = "2.64.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "27a59435a2597c355ff6bd6e0e2955043674ea5113d60e590dffecffda21540a" +checksum = "bdef727e13c2d26b5a431ea77d4727b30625d9eb9232f28ed4b7214a7065a719" dependencies = [ "either", "rowan", diff --git a/Cargo.toml b/Cargo.toml index 32f05cb..dfe6f94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "safe-migrate" -version = "0.8.0" +version = "0.8.1" edition = "2024" rust-version = "1.94" description = "Check PostgreSQL migrations against a synchronized database baseline" @@ -14,10 +14,10 @@ keywords = ["postgres", "migration", "linter", "ast", "database"] categories = ["command-line-utilities", "database"] [dependencies] -squawk-syntax = "=2.63.0" -squawk-lexer = "=2.63.0" -squawk-parser = "=2.63.0" -squawk-linter = "=2.63.0" +squawk-syntax = "=2.64.0" +squawk-lexer = "=2.64.0" +squawk-parser = "=2.64.0" +squawk-linter = "=2.64.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" toml = "0.8" diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index 210a067..ed926d1 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -20,7 +20,7 @@ database. | `sync` | Read PostgreSQL metadata and settings into a local cache. Requires `DATABASE_URL` and a local, Unix-socket, or tunneled connection. | | `cache inspect` | Show cache provenance and redacted counts without a database connection. Supports `--json`. | | `rules` | List primary rules; `--rule ` selects one and `--json` emits the discovery schema. | -| `init github-actions --path ` | Generate separate PR-analysis and trusted-refresh workflows. | +| `init github-actions --path ` | Generate separate PR-analysis and trusted-refresh workflows. With `--configure-secrets`, warn before secret setup when the baseline environment is missing, unverifiable, or lacks an access-protection rule. | | `init cache-key` | Generate a random 32-byte key as 64 lowercase hexadecimal characters. | `init github-actions --configure-secrets` sends the database URL and generated diff --git a/docs/GITHUB_ACTIONS.md b/docs/GITHUB_ACTIONS.md index 595206d..6f4e1b5 100644 --- a/docs/GITHUB_ACTIONS.md +++ b/docs/GITHUB_ACTIONS.md @@ -54,7 +54,10 @@ The command creates two files: It asks GitHub CLI for the database URL, generates a random 32-byte cache key, and sends both directly to GitHub. Neither value is printed or written to a -workflow file. +workflow file. Before setting the database URL, it checks the environment and +warns if GitHub reports no required reviewers or deployment-branch restriction, +or if the environment cannot be found or verified. The warning is non-blocking; +stop and protect the environment before answering GitHub CLI's secret prompt. The default branch is detected from `origin/HEAD`, with `main` as the fallback. Use `--branch ` when detection is wrong. This must be the default or PR diff --git a/src/_internal/ast/visitor.rs b/src/_internal/ast/visitor.rs index 5808cae..26751b2 100644 --- a/src/_internal/ast/visitor.rs +++ b/src/_internal/ast/visitor.rs @@ -139,10 +139,14 @@ impl AstVisitor { Stmt::CreateUser(node) => return Self::extract_create_user(node), Stmt::Begin(_) => return Some(StatementFact::BeginTransaction), Stmt::Commit(node) => { - return Some(if Self::is_and_chain(node.chain_clause()) { - StatementFact::CommitAndChain - } else { - StatementFact::CommitTransaction + return Some(match node { + ast::Commit::CommitPrepared(_) => StatementFact::OpaqueBlock, + ast::Commit::CommitTransaction(transaction) + if Self::is_and_chain(transaction.chain_clause()) => + { + StatementFact::CommitAndChain + } + ast::Commit::CommitTransaction(_) => StatementFact::CommitTransaction, }); } Stmt::Rollback(node) => return Self::extract_rollback(node), @@ -529,7 +533,9 @@ impl AstVisitor { continue; } if let Some(ac) = AlterConstraint::cast(action.syntax().clone()) { - let deferrable = ac.deferrable_constraint_option().is_some(); + let deferrable = ac.constraint_options().any(|option| { + matches!(option, ast::ConstraintOption::DeferrableConstraintOption(_)) + }); let name = ac .constraint_name_ref() .and_then(|name| name.path_ref()) @@ -959,6 +965,13 @@ impl AstVisitor { (columns, foreign_keys, table_constraints) } + fn column_constraints(col: &Column) -> impl Iterator + '_ { + col.clauses().filter_map(|clause| match clause { + ast::ColumnClause::ColumnConstraint(constraint) => Some(constraint), + _ => None, + }) + } + fn extract_column_fact(col: &Column) -> Option { let name_token = col.name().and_then(|n| n.ident_token()).or_else(|| { col.syntax() @@ -968,15 +981,14 @@ impl AstVisitor { })?; let name = Self::resolve_identifier_token(name_token.text()); let ty = col.ty().map(|t| t.syntax().text().to_string()); - let is_identity = col.constraints().any(|constraint| { + let is_identity = Self::column_constraints(col).any(|constraint| { matches!(constraint, ColumnConstraint::GeneratedConstraint(generated) if matches!(generated.generated_as(), Some(ast::GeneratedAs::GeneratedIdentity(_)))) }); let not_null = is_identity - || col - .constraints() + || Self::column_constraints(col) .any(|c| matches!(c, ColumnConstraint::NotNullConstraint(_))); - let primary_key_constraint_name = col.constraints().find_map(|constraint| { + let primary_key_constraint_name = Self::column_constraints(col).find_map(|constraint| { let ColumnConstraint::PrimaryKeyConstraint(primary_key) = constraint else { return None; }; @@ -990,7 +1002,7 @@ impl AstVisitor { }); let is_primary_key = primary_key_constraint_name.is_some(); let primary_key_constraint_name = primary_key_constraint_name.flatten(); - let unique_constraint_name = col.constraints().find_map(|constraint| { + let unique_constraint_name = Self::column_constraints(col).find_map(|constraint| { let ColumnConstraint::UniqueConstraint(unique) = constraint else { return None; }; @@ -1004,7 +1016,7 @@ impl AstVisitor { }); let is_unique = unique_constraint_name.is_some(); let unique_constraint_name = unique_constraint_name.flatten(); - let default = col.constraints().find_map(|c| { + let default = Self::column_constraints(col).find_map(|c| { if let ColumnConstraint::DefaultConstraint(dc) = c { Some(crate::_internal::analysis::expr_visitor::ExprVisitor::convert(dc.expr()?)) } else { @@ -1057,7 +1069,7 @@ impl AstVisitor { Some(AlterTableActionFact::DropNotNull { column: col_name }) } AlterColumnOption::SetType(st) => { - let has_using = st.using_token().is_some(); + let has_using = st.using_expr().is_some(); Some(AlterTableActionFact::SetType { column: col_name, ty: st.ty()?.syntax().text().to_string(), @@ -1378,7 +1390,7 @@ impl AstVisitor { }) }) .map(|t| Self::resolve_identifier_token(t.text())); - col.constraints() + Self::column_constraints(col) .filter_map(|c| { if let ColumnConstraint::ReferencesConstraint(rc) = c { let ref_path = rc @@ -2225,7 +2237,7 @@ impl AstVisitor { .and_then(|n| n.ident_token()) .map(|t| Self::resolve_identifier_token(t.text())), ty: param - .ty() + .func_type() .map(|t| t.syntax().text().to_string()) .unwrap_or_else(|| "unknown".into()), default: param.param_default().and_then(|pd| { @@ -2238,18 +2250,27 @@ impl AstVisitor { fn extract_ret_type( ret: &squawk_syntax::ast::RetType, ) -> crate::_internal::analysis::facts::RetTypeFact { - if let Some(tal) = ret.table_arg_list() { + if let Some(tal) = ret.return_table_arg_list() { let cols = tal .args() - .filter_map(|arg| match arg { - TableArg::Column(col) => Self::extract_column_fact(&col), - _ => None, + .filter_map(|column| { + Some(ColumnFact { + name: Self::resolve_ast_identifier(&column.name()?), + ty: column.func_type().map(|ty| ty.syntax().text().to_string()), + not_null: false, + is_primary_key: false, + primary_key_constraint_name: None, + is_unique: false, + unique_constraint_name: None, + default: None, + generation: crate::_internal::analysis::facts::ColumnGeneration::Ordinary, + }) }) .collect(); crate::_internal::analysis::facts::RetTypeFact::Table(cols) } else { let ty = ret - .ty() + .func_type() .map(|t| t.syntax().text().to_string()) .unwrap_or_else(|| "unknown".into()); crate::_internal::analysis::facts::RetTypeFact::Scalar(ty) @@ -2596,13 +2617,19 @@ impl AstVisitor { .map(|params| { params .params() + .chain( + params + .aggregate_order_by() + .into_iter() + .flat_map(|order_by| order_by.params()), + ) .filter_map(|param| { if matches!(param.mode(), Some(ast::ParamMode::ParamOut(_))) { return None; } Some( param - .ty() + .func_type() .map(|ty| ty.syntax().text().to_string()) .unwrap_or_else(|| "unknown".to_string()), ) @@ -3733,6 +3760,7 @@ impl AstVisitor { ast::ConfigValue::Literal(literal) => { Self::resolve_string_literal(&literal) } + ast::ConfigValue::PrefixExpr(_) => None, }) .collect(); (!schemas.is_empty()).then_some(StatementFact::SetSearchPath { @@ -3771,6 +3799,9 @@ impl AstVisitor { Self::resolve_string_literal(&literal) .or_else(|| Some(literal.syntax().text().to_string())) } + ast::ConfigValue::PrefixExpr(expr) => { + Some(expr.syntax().text().to_string()) + } }) .collect(); if values.len() != 1 { @@ -3894,20 +3925,20 @@ impl AstVisitor { } fn extract_rollback(node: &Rollback) -> Option { - if node.prepared_token().is_some() { - return Some(StatementFact::OpaqueBlock); - } - - match node - .savepoint_ref() - .and_then(|s| s.ident_token()) - .map(|t| Self::resolve_identifier_token(t.text())) - { - Some(name) => Some(StatementFact::RollbackToSavepoint { name }), - None if Self::is_and_chain(node.chain_clause()) => { + match node { + Rollback::RollbackPrepared(_) => Some(StatementFact::OpaqueBlock), + Rollback::RollbackToSavepoint(rollback) => rollback + .savepoint_ref() + .and_then(|savepoint| savepoint.ident_token()) + .map(|token| StatementFact::RollbackToSavepoint { + name: Self::resolve_identifier_token(token.text()), + }), + Rollback::RollbackTransaction(rollback) + if Self::is_and_chain(rollback.chain_clause()) => + { Some(StatementFact::RollbackAndChain) } - None => Some(StatementFact::RollbackTransaction), + Rollback::RollbackTransaction(_) => Some(StatementFact::RollbackTransaction), } } diff --git a/src/_internal/ast/visitor_tests.rs b/src/_internal/ast/visitor_tests.rs index d2f7ccf..5f079b0 100644 --- a/src/_internal/ast/visitor_tests.rs +++ b/src/_internal/ast/visitor_tests.rs @@ -4,8 +4,9 @@ mod tests { use crate::_internal::analysis::facts::{ AlterDatabaseAction, AlterPublicationActionFact, AlterSubscriptionActionFact, AlterTableActionFact, AlterTypeActionFact, PublicationObjectFact, PublicationScope, - ResetSettingTarget, SearchPathTarget, StatementFact, SubscriptionPublicationMode, - TableConstraintFact, TimeoutSetting, TimeoutSettingValue, TypeCreationKind, + ResetSettingTarget, RetTypeFact, SearchPathTarget, StatementFact, + SubscriptionPublicationMode, TableConstraintFact, TimeoutSetting, TimeoutSettingValue, + TypeCreationKind, }; use crate::_internal::ast::identifiers::{Ident, QualifiedName}; use crate::_internal::ast::visitor::AstVisitor; @@ -938,6 +939,20 @@ mod tests { )); } + #[test] + fn prepared_transaction_completion_does_not_end_the_current_transaction() { + for sql in [ + "COMMIT PREPARED 'migration';", + "ROLLBACK PREPARED 'migration';", + ] { + assert_eq!( + parse_and_extract_statement(sql), + Some(StatementFact::OpaqueBlock), + "{sql}" + ); + } + } + #[test] fn test_rollback_and_chain() { let facts = parse_and_extract_statement("ROLLBACK AND CHAIN;"); @@ -1111,6 +1126,14 @@ mod tests { local: false, }) )); + assert!(matches!( + parse_and_extract_statement("SET lock_timeout = -1;"), + Some(StatementFact::SetTimeout { + setting: TimeoutSetting::Lock, + value: TimeoutSettingValue::Invalid(_), + local: false, + }) + )); } #[test] @@ -1438,6 +1461,35 @@ mod tests { assert_eq!(legacy.params[0].ty, "integer"); } + #[test] + fn squawk_264_function_types_preserve_percent_and_table_signatures() { + let fact = parse_and_extract_statement( + "CREATE FUNCTION typed(value app.items.amount%TYPE) + RETURNS TABLE (label character varying(40), amount numeric, source app.items%TYPE) + LANGUAGE sql AS 'SELECT NULL, NULL, NULL';", + ) + .expect("typed function fact"); + let StatementFact::CreateFunction(function) = fact else { + panic!("expected create function fact"); + }; + assert_eq!(function.params.len(), 1); + assert_eq!(function.params[0].ty, "app.items.amount%TYPE"); + let Some(RetTypeFact::Table(columns)) = function.return_type else { + panic!("expected table return type"); + }; + assert_eq!( + columns + .iter() + .map(|column| (column.name.as_str(), column.ty.as_deref())) + .collect::>(), + [ + ("label", Some("character varying(40)")), + ("amount", Some("numeric")), + ("source", Some("app.items%TYPE")), + ] + ); + } + #[test] fn create_function_window_option_is_typed() { let fact = parse_and_extract_statement( @@ -2098,7 +2150,53 @@ mod tests { } #[test] - fn squawk_263_typed_alter_table_children_preserve_facts() { + fn squawk_264_alter_constraint_and_using_wrappers_preserve_facts() { + let facts = parse_and_extract( + "ALTER TABLE events ALTER CONSTRAINT events_parent_fk + DEFERRABLE INITIALLY DEFERRED; + ALTER TABLE events ALTER CONSTRAINT events_parent_fk + NOT DEFERRABLE INITIALLY IMMEDIATE; + ALTER TABLE events ALTER COLUMN payload TYPE jsonb USING payload::jsonb;", + ); + assert_eq!(facts.len(), 3); + assert!(matches!( + &facts[0], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::AlterConstraint { + name: Some(name), + deferrable: true, + }] if name == "events_parent_fk" + ) + )); + assert!(matches!( + &facts[1], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::AlterConstraint { + name: Some(name), + deferrable: false, + }] if name == "events_parent_fk" + ) + )); + assert!(matches!( + &facts[2], + StatementFact::AlterTable { actions, .. } + if matches!( + actions.as_slice(), + [AlterTableActionFact::SetType { + column, + ty, + has_using: true, + }] if column == "payload" && ty == "jsonb" + ) + )); + } + + #[test] + fn squawk_264_typed_alter_table_children_preserve_facts() { let facts = parse_and_extract( "ALTER TABLE events ADD COLUMN generated_id bigint GENERATED ALWAYS AS IDENTITY; ALTER TABLE events ADD CONSTRAINT events_parent_fk @@ -2392,7 +2490,7 @@ mod tests { } #[test] - fn squawk_263_typed_view_sequence_policy_and_function_children_preserve_facts() { + fn squawk_264_typed_view_sequence_policy_and_function_children_preserve_facts() { let facts = parse_and_extract( "CREATE SEQUENCE event_ids OWNED BY public.events.id; ALTER SEQUENCE event_ids OWNED BY NONE; @@ -2469,7 +2567,7 @@ mod tests { } #[test] - fn squawk_263_typed_replication_and_privilege_children_preserve_facts() { + fn squawk_264_typed_replication_and_privilege_children_preserve_facts() { let facts = parse_and_extract( "CREATE PUBLICATION all_events FOR ALL TABLES EXCEPT TABLE audit_events; CREATE PUBLICATION selected_events FOR TABLE public.events; @@ -2536,7 +2634,7 @@ mod tests { } #[test] - fn squawk_263_validation_matrix_rejects_invalid_expression_shapes() { + fn squawk_264_validation_matrix_rejects_invalid_expression_shapes() { for sql in [ "SELECT * FROM t WHERE a NOT IN ();", "SELECT * FROM t WHERE a NOT IN ARRAY[1, 2];", diff --git a/src/cli_init.rs b/src/cli_init.rs index 31718ff..15d532f 100644 --- a/src/cli_init.rs +++ b/src/cli_init.rs @@ -170,6 +170,67 @@ fn github_secret_args(name: &str, environment: Option<&str>) -> Vec { args } +fn github_environment_api_args(environment: &str) -> Vec { + vec![ + "api".to_string(), + format!("repos/{{owner}}/{{repo}}/environments/{environment}"), + ] +} + +fn github_environment_has_access_protection(response: &[u8]) -> Result { + let environment: serde_json::Value = + serde_json::from_slice(response).context("GitHub returned invalid environment metadata")?; + let protection_rules = environment + .get("protection_rules") + .and_then(serde_json::Value::as_array) + .context("GitHub environment metadata omitted protection_rules")?; + let has_review_or_branch_rule = protection_rules.iter().any(|rule| { + matches!( + rule.get("type").and_then(serde_json::Value::as_str), + Some("required_reviewers" | "branch_policy") + ) + }); + let has_deployment_branch_policy = + environment + .get("deployment_branch_policy") + .is_some_and(|policy| { + policy + .get("protected_branches") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false) + || policy + .get("custom_branch_policies") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false) + }); + Ok(has_review_or_branch_rule || has_deployment_branch_policy) +} + +fn warn_if_github_environment_is_unprotected(environment: &str) { + let output = Command::new("gh") + .args(github_environment_api_args(environment)) + .output(); + match output { + Ok(output) if output.status.success() => { + match github_environment_has_access_protection(&output.stdout) { + Ok(true) => {} + Ok(false) => eprintln!( + "Warning: GitHub environment `{environment}` has no required reviewers or deployment-branch restriction. Protect it before relying on SAFE_MIGRATE_DATABASE_URL isolation." + ), + Err(error) => eprintln!( + "Warning: could not verify protection for GitHub environment `{environment}`: {error}. Verify it before relying on SAFE_MIGRATE_DATABASE_URL isolation." + ), + } + } + Ok(_) => eprintln!( + "Warning: GitHub environment `{environment}` does not exist or could not be verified. `gh secret set --env` can create it without protection; create it and add required reviewers or a deployment-branch restriction before continuing." + ), + Err(error) => eprintln!( + "Warning: could not run `gh` to verify GitHub environment `{environment}`: {error}. Verify that it exists and is protected before continuing." + ), + } +} + fn set_github_secret(name: &str, value: Option<&str>, environment: Option<&str>) -> Result<()> { let mut command = Command::new("gh"); command.args(github_secret_args(name, environment)); @@ -299,6 +360,7 @@ fn run_github_actions( println!("Created {}", analysis_output.display()); println!("Created {}", baseline_output.display()); if configure_secrets { + warn_if_github_environment_is_unprotected(BASELINE_ENVIRONMENT); println!( "Enter SAFE_MIGRATE_DATABASE_URL for the {BASELINE_ENVIRONMENT} environment when GitHub CLI prompts for it." ); @@ -345,4 +407,35 @@ mod tests { ["secret", "set", "SAFE_MIGRATE_CACHE_KEY"] ); } + #[test] + fn environment_protection_requires_an_access_gate() { + assert_eq!( + github_environment_api_args(BASELINE_ENVIRONMENT), + [ + "api", + "repos/{owner}/{repo}/environments/safe-migrate-baseline" + ] + ); + + for protected in [ + br#"{"protection_rules":[{"type":"required_reviewers"}],"deployment_branch_policy":null}"#.as_slice(), + br#"{"protection_rules":[],"deployment_branch_policy":{"protected_branches":true,"custom_branch_policies":false}}"#.as_slice(), + br#"{"protection_rules":[],"deployment_branch_policy":{"protected_branches":false,"custom_branch_policies":true}}"#.as_slice(), + ] { + assert!(github_environment_has_access_protection(protected).unwrap()); + } + + assert!( + !github_environment_has_access_protection( + br#"{"protection_rules":[],"deployment_branch_policy":{"protected_branches":false,"custom_branch_policies":false}}"# + ) + .unwrap() + ); + assert!( + !github_environment_has_access_protection( + br#"{"protection_rules":[{"type":"wait_timer"}],"deployment_branch_policy":null}"# + ) + .unwrap() + ); + } } From 730416fb13bbc085d803738ac9894325890addde Mon Sep 17 00:00:00 2001 From: dsecurity49 Date: Sat, 5 Sep 2026 18:41:01 +0530 Subject: [PATCH 2/2] Keep tests out of the interactive terminal --- .github/ISSUE_TEMPLATE/database-feedback.yml | 2 +- CHANGELOG.md | 2 + README.md | 2 +- docs/BENCHMARKS.md | 2 +- docs/CONTRACT.md | 2 +- docs/GITHUB_ACTIONS.md | 2 +- scripts/test-action-contract | 2 +- src/_internal/report/interactive.rs | 54 +++++++++----------- 8 files changed, 32 insertions(+), 36 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/database-feedback.yml b/.github/ISSUE_TEMPLATE/database-feedback.yml index 3a9523f..33c3ff0 100644 --- a/.github/ISSUE_TEMPLATE/database-feedback.yml +++ b/.github/ISSUE_TEMPLATE/database-feedback.yml @@ -13,7 +13,7 @@ body: attributes: label: safe-migrate version description: Paste the output of `safe-migrate --version`. - placeholder: safe-migrate 0.8.0 + placeholder: safe-migrate 0.8.1 validations: required: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fba323..e65c851 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ notes are available on the migration transaction. - Warn before GitHub secret setup when the baseline environment is missing, unverifiable, or lacks reviewer or deployment-branch protection. +- Prevent `cargo test` from opening the interactive viewer when run from a + terminal. ## v0.8.0 — 2026-09-04 diff --git a/README.md b/README.md index fc76367..24595d8 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Prebuilt binaries are available from installer verifies release checksums: ```bash -VERSION='v0.8.0' +VERSION='v0.8.1' curl -fsSL "https://raw.githubusercontent.com/dsecurity49/safe-migrate/${VERSION}/install.sh" | bash -s -- --version "${VERSION}" ``` diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index ca113c6..266b266 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -1,7 +1,7 @@ # Local benchmark baseline This document records reproducible, non-CI performance scenarios. The values -are comparison points for the current `0.8.0` hardening work, not performance +are comparison points for the recorded `0.8.0` hardening work, not performance guarantees. Run them with: diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index ed926d1..fc532c5 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -1,6 +1,6 @@ # CLI and Report Contract -This document defines safe-migrate v0.8.0's CLI, report, cache, and GitHub +This document defines safe-migrate v0.8.1's CLI, report, cache, and GitHub Action behavior. If you are learning safe-migrate, start with the [README](../README.md). This diff --git a/docs/GITHUB_ACTIONS.md b/docs/GITHUB_ACTIONS.md index 6f4e1b5..57340ff 100644 --- a/docs/GITHUB_ACTIONS.md +++ b/docs/GITHUB_ACTIONS.md @@ -276,7 +276,7 @@ base commit: sparse-checkout: safe-migrate.toml persist-credentials: false -- uses: dsecurity49/safe-migrate@v0.8.0 +- uses: dsecurity49/safe-migrate@v0.8.1 env: SAFE_MIGRATE_CACHE_KEY: ${{ secrets.SAFE_MIGRATE_CACHE_KEY }} with: diff --git a/scripts/test-action-contract b/scripts/test-action-contract index 27d0db8..8922c97 100755 --- a/scripts/test-action-contract +++ b/scripts/test-action-contract @@ -9,7 +9,7 @@ baseline="$repo_root/scripts/action-baseline" manifest="$repo_root/action.yml" workflow="$repo_root/.github/workflows/ci.yml" -test "$(/bin/sh "$resolver" v0.8.0 "$repo_root/Cargo.toml")" = v0.8.0 +test "$(/bin/sh "$resolver" v0.8.1 "$repo_root/Cargo.toml")" = v0.8.1 test "$(/bin/sh "$resolver" 0123456789abcdef0123456789abcdef01234567 "$repo_root/Cargo.toml")" = source if /bin/sh "$resolver" main "$repo_root/Cargo.toml" >/dev/null 2>&1; then diff --git a/src/_internal/report/interactive.rs b/src/_internal/report/interactive.rs index b8431ef..88e821e 100644 --- a/src/_internal/report/interactive.rs +++ b/src/_internal/report/interactive.rs @@ -30,17 +30,22 @@ impl Drop for TerminalGuard { } } +fn require_interactive_terminal(stdin_is_terminal: bool, stdout_is_terminal: bool) -> Result<()> { + if !stdin_is_terminal || !stdout_is_terminal { + anyhow::bail!( + "Interactive mode requires a terminal connected to standard input and output" + ); + } + Ok(()) +} + pub fn run_interactive(violations: &[Violation], confidence: &Confidence) -> Result<()> { if violations.is_empty() { println!("No violations found!"); return Ok(()); } - if !stdin().is_terminal() || !stdout().is_terminal() { - anyhow::bail!( - "Interactive mode requires a terminal connected to standard input and output" - ); - } + require_interactive_terminal(stdin().is_terminal(), stdout().is_terminal())?; enable_raw_mode()?; let _guard = TerminalGuard; @@ -169,31 +174,20 @@ pub fn run_interactive(violations: &[Violation], confidence: &Confidence) -> Res #[cfg(test)] mod tests { use super::*; - use crate::_internal::report::violations::{ - ObjectKind, OperationKind, Violation, ViolationTier, - }; - #[test] - fn interactive_mode_explains_when_no_terminal_is_available() { - let violation = Violation { - source_range: None, - rule_id: "test-rule", - operation_kind: OperationKind::Other("test".to_string()), - object_kind: ObjectKind::Unknown, - object_name: "test".to_string(), - tier: ViolationTier::Tier3, - reason: "test".to_string(), - recipe: "test", - dedup_key: None, - sql: None, - fk_dependency_related: false, - }; - - let error = run_interactive(&[violation], &Confidence::Exact).unwrap_err(); - assert!( - error - .to_string() - .contains("requires a terminal connected to standard input and output") - ); + fn interactive_mode_explains_when_terminal_is_incomplete() { + for (stdin_is_terminal, stdout_is_terminal) in + [(false, false), (false, true), (true, false)] + { + let error = + require_interactive_terminal(stdin_is_terminal, stdout_is_terminal).unwrap_err(); + assert!( + error + .to_string() + .contains("requires a terminal connected to standard input and output") + ); + } + + assert!(require_interactive_terminal(true, true).is_ok()); } }