From e952bb4c7fa4e92057e9e077a47b318fe6786b50 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 2 Jun 2026 18:41:36 -0400 Subject: [PATCH] fix(config): disable Slack unless configured Require an explicit server.integrations.slack table before Slack reports enabled or starts from vault tokens. --- .../administration/server-configuration.mdx | 4 +- docs/public/human-tools/interviews.mdx | 2 +- docs/public/integrations/slack.mdx | 3 +- lib/crates/fabro-config/src/resolve/server.rs | 5 +- .../fabro-config/src/tests/resolve_server.rs | 18 ++++++- lib/crates/fabro-server/src/server/tests.rs | 50 +++++++++++++++++-- .../fabro-server/tests/it/api/system.rs | 35 +++++++++++++ 7 files changed, 107 insertions(+), 10 deletions(-) diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index 8c6139415..a1ffdcd9a 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -444,9 +444,9 @@ GitHub App mode stores these secrets in the vault. `fabro install` writes them a ### Slack integration (optional) -Slack credentials are server-level secrets. They enable one Slack connection that is shared by human interview prompts and run lifecycle notifications. `server.integrations.slack.default_channel` is optional and is used only as the default destination for interview prompts; lifecycle notifications use `[run.notifications..slack].channel` in run or workflow configuration. +Slack credentials are server-level secrets. Add `[server.integrations.slack]` to enable one Slack connection that is shared by human interview prompts and run lifecycle notifications. `server.integrations.slack.default_channel` is optional and is used only as the default destination for interview prompts; lifecycle notifications use `[run.notifications..slack].channel` in run or workflow configuration. -Fabro resolves these from the vault only. If both are present, startup logs `Slack integration enabled` and then the Slack Socket Mode connection status. If either is missing or empty, startup logs `Slack integration disabled; missing credentials` with the missing variable names. +Fabro resolves these from the vault only. When `[server.integrations.slack]` is present and both credentials are present, startup logs `Slack integration enabled` and then the Slack Socket Mode connection status. If the Slack config table is absent or `enabled = false`, startup logs `Slack integration disabled by server configuration`. If the table is present but either credential is missing or empty, startup logs `Slack integration disabled; missing credentials` with the missing variable names. ```bash fabro secret set FABRO_SLACK_BOT_TOKEN xoxb-... diff --git a/docs/public/human-tools/interviews.mdx b/docs/public/human-tools/interviews.mdx index 6dadc9eea..e15e1b2d2 100644 --- a/docs/public/human-tools/interviews.mdx +++ b/docs/public/human-tools/interviews.mdx @@ -94,7 +94,7 @@ If the pending session disappears before an answer is submitted, the waiting que Fabro's [Slack integration](/integrations/slack) uses the web interviewer under the hood. When a human gate fires, the pending question is rendered as a Slack message with interactive buttons. When a user clicks a button, the Slack event handler calls `submit_answer()` on the web interviewer, unblocking the workflow. -Slack interview prompts require Slack server credentials and a destination channel. Most servers should configure `server.integrations.slack.default_channel`; lifecycle notifications use their own per-route channels instead. +Slack interview prompts require Slack server credentials, an enabled `[server.integrations.slack]` table, and a destination channel. Most servers should configure `server.integrations.slack.default_channel`; lifecycle notifications use their own per-route channels instead. ### Auto-approve diff --git a/docs/public/integrations/slack.mdx b/docs/public/integrations/slack.mdx index 43897ff2e..bca381e79 100644 --- a/docs/public/integrations/slack.mdx +++ b/docs/public/integrations/slack.mdx @@ -109,10 +109,11 @@ The server runtime does not read Slack tokens from process env or `server.env`. Restart the server after changing Slack credentials so the Socket Mode connection is recreated with the new tokens. -Optionally, set a default interview channel in your [server configuration](/administration/server-configuration): +Enable Slack in your [server configuration](/administration/server-configuration). You can leave `default_channel` out if you only use per-route lifecycle notification channels: ```toml title="settings.toml" [server.integrations.slack] +enabled = true default_channel = "#fabro-reviews" ``` diff --git a/lib/crates/fabro-config/src/resolve/server.rs b/lib/crates/fabro-config/src/resolve/server.rs index 9a2538295..664cceca2 100644 --- a/lib/crates/fabro-config/src/resolve/server.rs +++ b/lib/crates/fabro-config/src/resolve/server.rs @@ -330,7 +330,10 @@ fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegr enabled: slack.enabled.unwrap_or(true), default_channel: slack.default_channel.clone(), }) - .unwrap_or_default(), + .unwrap_or(SlackIntegrationSettings { + enabled: false, + default_channel: None, + }), } } diff --git a/lib/crates/fabro-config/src/tests/resolve_server.rs b/lib/crates/fabro-config/src/tests/resolve_server.rs index be646e73c..d45f1e040 100644 --- a/lib/crates/fabro-config/src/tests/resolve_server.rs +++ b/lib/crates/fabro-config/src/tests/resolve_server.rs @@ -110,7 +110,7 @@ fn resolves_server_defaults_from_empty_settings() { } #[test] -fn resolved_server_integrations_are_slack_only_for_chat() { +fn resolved_server_integrations_disable_slack_when_config_is_absent() { let settings = resolve_server(&empty_settings_with_auth_methods()); let integrations = @@ -128,13 +128,27 @@ fn resolved_server_integrations_are_slack_only_for_chat() { "webhooks": null, }, "slack": { - "enabled": true, + "enabled": false, "default_channel": null, }, }) ); } +#[test] +fn resolved_server_integrations_enable_slack_when_config_is_present() { + let settings = resolve_server(&parse( + r" +_version = 1 + +[server.integrations.slack] +", + )); + + assert!(settings.integrations.slack.enabled); + assert!(settings.integrations.slack.default_channel.is_none()); +} + #[test] fn server_sandbox_defaults_all_providers_enabled() { let settings = ServerSettingsBuilder::from_toml( diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 1076b3f7e..363884738 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -1929,6 +1929,18 @@ fn server_secrets_resolve_bootstrap_process_env_before_server_env() { fn slack_app_state_with_secret_sources( vault_entries: &[(&str, &str, SecretType)], server_secret_env: HashMap, +) -> Arc { + slack_app_state_with_settings_and_secret_sources( + default_test_server_settings(), + vault_entries, + server_secret_env, + ) +} + +fn slack_app_state_with_settings_and_secret_sources( + settings: ServerSettings, + vault_entries: &[(&str, &str, SecretType)], + server_secret_env: HashMap, ) -> Arc { let (store, artifact_store) = test_store_bundle(); let vault_path = test_secret_store_path(); @@ -1939,7 +1951,7 @@ fn slack_app_state_with_secret_sources( } build_app_state(AppStateConfig { resolved_settings: resolved_runtime_settings_for_tests( - default_test_server_settings(), + settings, RunLayer::default(), LlmCatalogSettings::default(), ), @@ -1965,7 +1977,7 @@ fn slack_app_state_with_secret_sources( } #[test] -fn slack_service_is_enabled_by_vault_tokens() { +fn slack_service_ignores_vault_tokens_when_config_is_absent() { let state = slack_app_state_with_secret_sources( &[ ( @@ -1982,10 +1994,42 @@ fn slack_service_is_enabled_by_vault_tokens() { HashMap::new(), ); + assert!(state.slack_service.is_none()); +} + +#[test] +fn slack_service_is_enabled_by_config_and_vault_tokens() { + let state = slack_app_state_with_settings_and_secret_sources( + server_settings_from_toml( + r#" +_version = 1 + +[server.auth] +methods = ["dev-token"] + +[server.integrations.slack] +enabled = true +"#, + ), + &[ + ( + EnvVars::FABRO_SLACK_BOT_TOKEN, + "xoxb-test", + SecretType::Token, + ), + ( + EnvVars::FABRO_SLACK_APP_TOKEN, + "xapp-test", + SecretType::Token, + ), + ], + HashMap::new(), + ); + let service = state .slack_service .as_ref() - .expect("slack service should be enabled by vault tokens"); + .expect("slack service should be enabled by config and vault tokens"); let connection = service.connection_status(); assert_eq!(connection.kind, IntegrationConnectionKind::SocketMode); assert_eq!(connection.status, IntegrationConnectionState::Connecting); diff --git a/lib/crates/fabro-server/tests/it/api/system.rs b/lib/crates/fabro-server/tests/it/api/system.rs index b37c48f25..1604d49cd 100644 --- a/lib/crates/fabro-server/tests/it/api/system.rs +++ b/lib/crates/fabro-server/tests/it/api/system.rs @@ -145,6 +145,41 @@ async fn get_system_info_returns_runtime_fields() { ); } +#[tokio::test] +async fn get_system_integrations_reports_slack_disabled_when_config_is_absent() { + let settings = settings_from_toml( + r" +_version = 1 +", + ); + let app = fabro_server::test_support::build_test_router( + fabro_server::test_support::TestAppStateBuilder::new() + .runtime_settings(settings.server_settings, settings.manifest_run_defaults) + .build(), + ); + + let request = Request::builder() + .method("GET") + .uri(api("/system/integrations")) + .body(Body::empty()) + .unwrap(); + let response = app.oneshot(request).await.unwrap(); + + let body = response_json(response, StatusCode::OK, "GET /api/v1/system/integrations").await; + let slack = body["data"] + .as_array() + .expect("integration response should include data") + .iter() + .find(|integration| integration["provider"] == "slack") + .expect("slack integration should be present"); + + assert_eq!(slack["enabled"], false); + assert_eq!(slack["configured"], false); + assert_eq!(slack["status"], "disabled"); + assert_eq!(slack["missing_credentials"], serde_json::json!([])); + assert!(slack["connection"].is_null()); +} + #[tokio::test] async fn get_system_integrations_reports_slack_missing_credentials_when_allowed() { let settings = settings_from_toml(