From cf93ef88045b40da4e055bbd50de11f0c0dd2fa6 Mon Sep 17 00:00:00 2001 From: Chris Raethke Date: Thu, 17 Sep 2026 18:05:08 +1000 Subject: [PATCH 1/3] chore: add Aislop quality gate --- .aislop/config.yml | 37 +++++++++++++++++++++ .aislop/rules.yml | 14 ++++++++ .github/workflows/aislop.yml | 15 +++++++++ .gitignore | 4 ++- src/api/client.rs | 16 ++++------ src/api/edge.rs | 4 +-- src/api/web.rs | 40 ----------------------- src/auth/browser.rs | 4 +-- src/auth/extract/chromium.rs | 14 +++----- src/auth/extract/crypto.rs | 12 ------- src/auth/oauth.rs | 39 ++++++++++++++--------- src/auth/resolve.rs | 16 +++++----- src/auth/storage.rs | 62 ++++++++++++++++++++++++++++-------- src/auth/tokens.rs | 37 +++++++++++---------- src/cli/auth.rs | 31 ++++++++++++------ src/main.rs | 4 --- 16 files changed, 207 insertions(+), 142 deletions(-) create mode 100644 .aislop/config.yml create mode 100644 .aislop/rules.yml create mode 100644 .github/workflows/aislop.yml diff --git a/.aislop/config.yml b/.aislop/config.yml new file mode 100644 index 0000000..dbff164 --- /dev/null +++ b/.aislop/config.yml @@ -0,0 +1,37 @@ +version: 1 +engines: + format: true + lint: true + code-quality: true + ai-slop: true + architecture: true + security: true +quality: + maxFunctionLoc: 80 + maxFileLoc: 400 + maxNesting: 5 + maxParams: 6 +lint: + typecheck: true + expoDoctor: false +security: + audit: true + auditTimeout: 25000 +scoring: + weights: + format: 0.3 + lint: 0.6 + code-quality: 0.8 + ai-slop: 1 + architecture: 1 + security: 1.5 + thresholds: + good: 75 + ok: 50 + smoothing: 5 + maxPerRule: 40 +ci: + failBelow: 85 + format: json +telemetry: + enabled: true diff --git a/.aislop/rules.yml b/.aislop/rules.yml new file mode 100644 index 0000000..5e3772e --- /dev/null +++ b/.aislop/rules.yml @@ -0,0 +1,14 @@ +# Architecture rules (BYO) +# Uncomment and customize to enforce your project's conventions. +# +# rules: +# - name: no-axios +# type: forbid_import +# match: "axios" +# severity: error +# +# - name: controller-no-db +# type: forbid_import_from_path +# from: "src/controllers/**" +# forbid: "src/db/**" +# severity: error diff --git a/.github/workflows/aislop.yml b/.github/workflows/aislop.yml new file mode 100644 index 0000000..555d1f0 --- /dev/null +++ b/.github/workflows/aislop.yml @@ -0,0 +1,15 @@ +name: aislop + +on: + push: + branches: [main] + pull_request: + +jobs: + quality-gate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: scanaislop/aislop@v1 + with: + version: latest diff --git a/.gitignore b/.gitignore index 37eb6b7..1ca489f 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,6 @@ run-ralph-loop.sh /.claude /.codex /.pi -/.aislop +/.aislop/* +!/.aislop/config.yml +!/.aislop/rules.yml diff --git a/src/api/client.rs b/src/api/client.rs index 714096b..ce50f4f 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -582,7 +582,7 @@ mod tests { match token_type { TokenType::UserOAuth => TokenSet { token_type: TokenType::UserOAuth, - access_token: "xoxp-123456789-0123456789-abcdef".to_string(), + access_token: "xoxp-123456789-0123456789-abcdef".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture xoxd_cookie: None, team_id: "T12345".to_string(), team_name: "Test".to_string(), @@ -593,7 +593,7 @@ mod tests { }, TokenType::BotOAuth => TokenSet { token_type: TokenType::BotOAuth, - access_token: "xoxb-123456789-0123456789-abcdef".to_string(), + access_token: "xoxb-123456789-0123456789-abcdef".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture xoxd_cookie: None, team_id: "T12345".to_string(), team_name: "Test".to_string(), @@ -604,7 +604,7 @@ mod tests { }, TokenType::Browser => TokenSet { token_type: TokenType::Browser, - access_token: "xoxc-123456789-0123456789-abcdef".to_string(), + access_token: "xoxc-123456789-0123456789-abcdef".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic browser fixture xoxd_cookie: Some("xoxd-test-cookie".to_string()), team_id: "T12345".to_string(), team_name: "Test".to_string(), @@ -812,8 +812,6 @@ mod tests { assert!(client.base_url().contains("slack.com") || client.base_url().starts_with("http")); } - // --- normalize_api_endpoint / api_request validation ------------------- - #[test] fn test_normalize_endpoint_bare_method_names() { for m in [ @@ -874,10 +872,10 @@ mod tests { "ftp://slack.com/api/auth.test", "file:///etc/passwd", // Wrong host / lookalikes - "https://evil.com/api/auth.test", - "https://slack.com.evil.com/api/auth.test", + "https://evil.com/api/auth.test", // aislop-ignore-line ai-slop/hardcoded-url -- invalid-host validation fixture + "https://slack.com.evil.com/api/auth.test", // aislop-ignore-line ai-slop/hardcoded-url -- lookalike-host validation fixture "https://api.slack.com/api/auth.test", - "https://slack.com@evil.com/api/auth.test", + "https://slack.com@evil.com/api/auth.test", // aislop-ignore-line ai-slop/hardcoded-url -- userinfo validation fixture // Userinfo "https://user:pass@slack.com/api/auth.test", "https://user@slack.com/api/auth.test", @@ -914,7 +912,7 @@ mod tests { let client = SlackClient::with_base_url(token, "http://127.0.0.1:1".to_string()).unwrap(); let err = client .api_request( - "https://evil.com/api/auth.test", + "https://evil.com/api/auth.test", // aislop-ignore-line ai-slop/hardcoded-url -- invalid-host validation fixture reqwest::Method::GET, &serde_json::Value::Null, ) diff --git a/src/api/edge.rs b/src/api/edge.rs index 3bff9e0..6f0da35 100644 --- a/src/api/edge.rs +++ b/src/api/edge.rs @@ -323,7 +323,7 @@ mod tests { fn create_browser_token() -> TokenSet { TokenSet { token_type: TokenType::Browser, - access_token: "xoxc-1234567890-0123456789-abcdefghij".to_string(), + access_token: "xoxc-1234567890-0123456789-abcdefghij".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic browser fixture xoxd_cookie: Some("xoxd-test-cookie-value".to_string()), team_id: "T12345".to_string(), team_name: "Test Team".to_string(), @@ -337,7 +337,7 @@ mod tests { fn create_oauth_token() -> TokenSet { TokenSet { token_type: TokenType::UserOAuth, - access_token: "xoxp-1234567890-0123456789-abcdefghij".to_string(), + access_token: "xoxp-1234567890-0123456789-abcdefghij".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture xoxd_cookie: None, team_id: "T12345".to_string(), team_name: "Test Team".to_string(), diff --git a/src/api/web.rs b/src/api/web.rs index bb911b1..6f17208 100644 --- a/src/api/web.rs +++ b/src/api/web.rs @@ -20,10 +20,6 @@ use super::types::{ /// Maximum file download size (5MB) const MAX_FILE_SIZE: u64 = 5 * 1024 * 1024; -// ============================================================================ -// Auth Methods -// ============================================================================ - impl SlackClient { /// Test authentication and get information about the token /// @@ -36,10 +32,6 @@ impl SlackClient { } } -// ============================================================================ -// Conversations Methods -// ============================================================================ - /// Parameters for conversations.list #[derive(Debug, Serialize, Default)] pub struct ConversationsListParams { @@ -367,10 +359,6 @@ impl SlackClient { } } -// ============================================================================ -// Chat Methods -// ============================================================================ - /// Parameters for chat.postMessage #[derive(Debug, Serialize)] pub struct ChatPostMessageParams { @@ -441,10 +429,6 @@ impl SlackClient { } } -// ============================================================================ -// Search Methods -// ============================================================================ - /// Parameters for search.messages #[derive(Debug, Serialize)] pub struct SearchMessagesParams { @@ -505,10 +489,6 @@ impl SlackClient { } } -// ============================================================================ -// Users Methods -// ============================================================================ - impl SlackClient { /// List all users in the workspace /// @@ -562,10 +542,6 @@ impl SlackClient { } } -// ============================================================================ -// Reactions Methods -// ============================================================================ - impl SlackClient { /// Add a reaction to a message /// @@ -618,10 +594,6 @@ impl SlackClient { } } -// ============================================================================ -// Files Methods -// ============================================================================ - impl SlackClient { /// Get information about a file /// @@ -700,10 +672,6 @@ impl SlackClient { } } -// ============================================================================ -// Reactions Get Method -// ============================================================================ - impl SlackClient { /// Get reactions for a message /// @@ -731,10 +699,6 @@ impl SlackClient { } } -// ============================================================================ -// Reminders Methods -// ============================================================================ - impl SlackClient { /// List reminders /// @@ -802,10 +766,6 @@ impl SlackClient { } } -// ============================================================================ -// Status/Presence Methods -// ============================================================================ - impl SlackClient { /// Get the current user's profile /// diff --git a/src/auth/browser.rs b/src/auth/browser.rs index e14e505..cc0c826 100644 --- a/src/auth/browser.rs +++ b/src/auth/browser.rs @@ -179,7 +179,7 @@ mod tests { #[test] fn test_browser_tokens_invalid_xoxc_prefix() { let tokens = BrowserTokens::new( - "xoxp-1234567890-abcdef123456789012345678901234567890".into(), + "xoxp-1234567890-abcdef123456789012345678901234567890".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic invalid-prefix fixture "xoxd-abcdefghijklmnopqrstuvwxyz1234567890".into(), ); @@ -249,7 +249,7 @@ mod tests { // but should be long encoded values let tokens = BrowserTokens::new( "xoxc-1234567890-abcdef123456789012345678901234567890".into(), - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c".into(), + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic legacy cookie fixture ); // Long JWT-like cookie should be accepted diff --git a/src/auth/extract/chromium.rs b/src/auth/extract/chromium.rs index 4468298..cc7dacf 100644 --- a/src/auth/extract/chromium.rs +++ b/src/auth/extract/chromium.rs @@ -118,10 +118,6 @@ pub fn extract_tokens_from_leveldb(leveldb_dir: &Path) -> Result> Ok(parse_recovered(&recovered)) } -// --------------------------------------------------------------------------- -// Minimal LevelDB SSTable reader -// --------------------------------------------------------------------------- - /// Read an unsigned LEB128 varint, advancing `pos`. Returns `None` on overflow /// or truncation. fn read_varint(buf: &[u8], pos: &mut usize) -> Option { @@ -252,10 +248,6 @@ fn sstable_data_bytes(file: &[u8]) -> Option> { Some(out) } -// --------------------------------------------------------------------------- -// Token parsing -// --------------------------------------------------------------------------- - /// Parse recovered bytes across multiple textual "views" and merge the results. /// /// Chromium encodes non-ASCII Local Storage values as UTF-16LE, which litters @@ -866,7 +858,7 @@ mod tests { assert!(parse_local_config("localConfig_v2 no object").is_empty()); assert!(parse_local_config("localConfig_v2{broken}").is_empty()); assert!(parse_local_config( - "localConfig_v2{\"teams\":{\"T1234567\":{\"token\":\"xoxp-not-client\"},\"T7654321\":{}}}}" + "localConfig_v2{\"teams\":{\"T1234567\":{\"token\":\"xoxp-not-client\"},\"T7654321\":{}}}}" // aislop-ignore-line security/hardcoded-secret -- synthetic parser fixture ) .is_empty()); @@ -917,7 +909,9 @@ mod tests { #[test] fn nested_object_and_team_id_scans_are_bounded() { let text = r#"{"outer":{"closed":{}} ,"T1234567":{"token":"xoxc-nested-object-1234"}}"#; - let pos = text.find("xoxc-").unwrap(); + let pos = text + .find("xoxc-") + .expect("fixture contains an xoxc token marker"); let (start, end) = enclosing_object(text.as_bytes(), pos); assert_eq!(&text[start..=end], r#"{"token":"xoxc-nested-object-1234"}"#); assert_eq!( diff --git a/src/auth/extract/crypto.rs b/src/auth/extract/crypto.rs index d95e9f5..1a94377 100644 --- a/src/auth/extract/crypto.rs +++ b/src/auth/extract/crypto.rs @@ -52,10 +52,6 @@ const XOXD_PREFIX: &str = "xoxd-"; /// AES block size / derived-key length in bytes. const AES_KEY_LEN: usize = 16; -// --------------------------------------------------------------------------- -// safe_storage_key -// --------------------------------------------------------------------------- - /// Derive every candidate AES key that might decrypt a profile's cookie values. /// /// On macOS a `" Safe Storage"` service can contain more than one generic @@ -179,10 +175,6 @@ fn derive_key(password: &[u8]) -> Vec { key } -// --------------------------------------------------------------------------- -// decrypt_cookie_value -// --------------------------------------------------------------------------- - /// Decrypt a Chromium `encrypted_value` blob into a Slack `xoxd-…` cookie. /// /// The `encrypted_value` must start with the `b"v10"` marker. The remaining @@ -252,10 +244,6 @@ fn finalize_cookie_plaintext(plaintext: Vec) -> Result { )) } -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - #[cfg(test)] mod tests { use super::*; diff --git a/src/auth/oauth.rs b/src/auth/oauth.rs index 50f1bc0..6753068 100644 --- a/src/auth/oauth.rs +++ b/src/auth/oauth.rs @@ -422,7 +422,7 @@ fn start_callback_server( // Accept one request if let Some(request) = server.recv_timeout(Duration::from_secs(120))? { - let url_str = format!("http://localhost{}", request.url()); + let url_str = format!("http://localhost{}", request.url()); // aislop-ignore-line ai-slop/hardcoded-url -- callback server is intentionally loopback-only let url = match Url::parse(&url_str) { Ok(u) => u, Err(e) => { @@ -595,7 +595,7 @@ mod tests { fn test_parse_oauth_response_success_bot_token() { let body: serde_json::Value = serde_json::json!({ "ok": true, - "access_token": "xoxb-123456789-0123456789-abcdefghijklmnop", + "access_token": "xoxb-123456789-0123456789-abcdefghijklmnop", // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture "token_type": "bot", "scope": "channels:read,users:read", "bot_user_id": "UBOT12345", @@ -611,6 +611,7 @@ mod tests { let token_set = result.unwrap(); assert_eq!( token_set.access_token, + // aislop-ignore-next-line security/hardcoded-secret -- synthetic OAuth response fixture "xoxb-123456789-0123456789-abcdefghijklmnop" ); assert_eq!(token_set.team_id, "T12345678"); @@ -624,12 +625,12 @@ mod tests { fn test_parse_oauth_response_success_user_token() { let body: serde_json::Value = serde_json::json!({ "ok": true, - "access_token": "xoxp-123456789-0123456789-0123456789-abcdef", + "access_token": "xoxp-123456789-0123456789-0123456789-abcdef", // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture "token_type": "user", "scope": "channels:read,chat:write", "authed_user": { "id": "U12345678", - "access_token": "xoxp-123456789-0123456789-0123456789-abcdef" + "access_token": "xoxp-123456789-0123456789-0123456789-abcdef" // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture }, "team": { "id": "T98765432", @@ -643,6 +644,7 @@ mod tests { let token_set = result.unwrap(); assert_eq!( token_set.access_token, + // aislop-ignore-next-line security/hardcoded-secret -- synthetic OAuth response fixture "xoxp-123456789-0123456789-0123456789-abcdef" ); assert_eq!(token_set.team_id, "T98765432"); @@ -679,7 +681,7 @@ mod tests { "scope": "search:read", "authed_user": { "id": "UUSER1234", - "access_token": "xoxp-user-token-here-abcdef123" + "access_token": "xoxp-user-token-here-abcdef123" // aislop-ignore-line security/hardcoded-secret -- synthetic fallback fixture }, "team": { "id": "TUSER1234", @@ -691,7 +693,10 @@ mod tests { assert!(result.is_ok()); let token_set = result.unwrap(); - assert_eq!(token_set.access_token, "xoxp-user-token-here-abcdef123"); + assert_eq!( + token_set.access_token, + "xoxp-user-token-here-abcdef123" // aislop-ignore-line security/hardcoded-secret -- synthetic fallback fixture + ); assert_eq!(token_set.user_id, "UUSER1234"); assert_eq!(token_set.token_type, crate::auth::TokenType::UserOAuth); } @@ -701,7 +706,7 @@ mod tests { // Test case where neither user_id nor bot_user_id is present let body: serde_json::Value = serde_json::json!({ "ok": true, - "access_token": "xoxb-minimal-token-here-1234", + "access_token": "xoxb-minimal-token-here-1234", // aislop-ignore-line security/hardcoded-secret -- synthetic fallback fixture "token_type": "bot", "scope": "", "team": { @@ -751,6 +756,7 @@ mod tests { #[test] fn exchange_code_posts_form_and_maps_success() { use mockito::Matcher; + let expected_token = "xoxb-test-access-token"; let mut server = mockito::Server::new(); let mock = server @@ -768,13 +774,14 @@ mod tests { .with_status(200) .with_header("content-type", "application/json") .with_body( - r#"{ + serde_json::json!({ "ok": true, - "access_token": "xoxb-test-access-token", + "access_token": &expected_token, "scope": "channels:read,chat:write", "bot_user_id": "UBOT", "team": {"id": "TTEAM", "name": "OAuth Team"} - }"#, + }) + .to_string(), ) .create(); let flow = test_flow(format!("{}/oauth.v2.access", server.url()), 9123); @@ -782,7 +789,7 @@ mod tests { let token = flow.exchange_code("oauth-code").unwrap(); mock.assert(); - assert_eq!(token.access_token, "xoxb-test-access-token"); + assert_eq!(token.access_token, expected_token); assert_eq!(token.team_id, "TTEAM"); assert_eq!(token.team_name, "OAuth Team"); assert_eq!(token.user_id, "UBOT"); @@ -916,6 +923,7 @@ mod tests { fn oauth_success_mock(server: &mut mockito::Server, expected_code: &str) -> mockito::Mock { use mockito::Matcher; + let expected_token = "xoxp-manual-access-token"; // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture server .mock("POST", "/oauth.v2.access") @@ -926,13 +934,14 @@ mod tests { .with_status(200) .with_header("content-type", "application/json") .with_body( - r#"{ + serde_json::json!({ "ok": true, - "access_token": "xoxp-manual-access-token", + "access_token": &expected_token, "scope": "users:read", "authed_user": {"id": "UMANUAL"}, "team": {"id": "TMANUAL", "name": "Manual Team"} - }"#, + }) + .to_string(), ) .create() } @@ -953,7 +962,7 @@ mod tests { .unwrap(); mock.assert(); - assert_eq!(token.access_token, "xoxp-manual-access-token"); + assert_eq!(token.access_token, "xoxp-manual-access-token"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture assert!(presented_url.unwrap().contains("state=known-state")); } diff --git a/src/auth/resolve.rs b/src/auth/resolve.rs index 3a185db..af7c341 100644 --- a/src/auth/resolve.rs +++ b/src/auth/resolve.rs @@ -64,9 +64,9 @@ mod tests { #[test] fn test_override_user_oauth_token() { - let token = resolve_token(None, Some("xoxp-1234567890-abcdef")).unwrap(); + let token = resolve_token(None, Some("xoxp-1234567890-abcdef")).unwrap(); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture assert_eq!(token.token_type, TokenType::UserOAuth); - assert_eq!(token.access_token, "xoxp-1234567890-abcdef"); + assert_eq!(token.access_token, "xoxp-1234567890-abcdef"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture assert_eq!(token.team_id, "unknown"); assert_eq!(token.team_name, "unknown"); assert_eq!(token.user_id, "unknown"); @@ -76,9 +76,9 @@ mod tests { #[test] fn test_override_bot_oauth_token() { - let token = resolve_token(None, Some("xoxb-1234567890-abcdef")).unwrap(); + let token = resolve_token(None, Some("xoxb-1234567890-abcdef")).unwrap(); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture assert_eq!(token.token_type, TokenType::BotOAuth); - assert_eq!(token.access_token, "xoxb-1234567890-abcdef"); + assert_eq!(token.access_token, "xoxb-1234567890-abcdef"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture } #[test] @@ -87,15 +87,15 @@ mod tests { // never consulted (a nonexistent workspace would otherwise error). let token = resolve_token( Some("definitely-not-a-real-workspace"), - Some("xoxp-1234567890-abcdef"), + Some("xoxp-1234567890-abcdef"), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture ) .unwrap(); - assert_eq!(token.access_token, "xoxp-1234567890-abcdef"); + assert_eq!(token.access_token, "xoxp-1234567890-abcdef"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture } #[test] fn test_override_browser_token_rejected() { - let err = resolve_token(None, Some("xoxc-1234567890-abcdef")).unwrap_err(); + let err = resolve_token(None, Some("xoxc-1234567890-abcdef")).unwrap_err(); // aislop-ignore-line security/hardcoded-secret -- synthetic browser fixture match err { SlackError::InvalidToken(msg) => { assert!(msg.contains("--xoxc and --xoxd"), "unexpected msg: {}", msg); @@ -122,7 +122,7 @@ mod tests { #[test] fn test_override_malformed_token_rejected() { // Valid prefix but invalid characters fails TokenSet validation. - let err = resolve_token(None, Some("xoxp-bad token!")).unwrap_err(); + let err = resolve_token(None, Some("xoxp-bad token!")).unwrap_err(); // aislop-ignore-line security/hardcoded-secret -- synthetic malformed fixture assert!(matches!(err, SlackError::InvalidToken(_))); } } diff --git a/src/auth/storage.rs b/src/auth/storage.rs index c2ec856..0e881df 100644 --- a/src/auth/storage.rs +++ b/src/auth/storage.rs @@ -478,44 +478,77 @@ mod tests { fn seed(&self, key: &str, value: impl Into) { self.entries .lock() - .unwrap() + .expect("memory store entries lock poisoned") .insert(key.to_string(), value.into()); } fn value(&self, key: &str) -> Option { - self.entries.lock().unwrap().get(key).cloned() + self.entries + .lock() + .expect("memory store entries lock poisoned") + .get(key) + .cloned() } fn operations(&self) -> Vec { - self.operations.lock().unwrap().clone() + self.operations + .lock() + .expect("memory store operations lock poisoned") + .clone() } fn fail_get(&self, key: &str) { - self.failing_gets.lock().unwrap().insert(key.to_string()); + self.failing_gets + .lock() + .expect("memory store failing-gets lock poisoned") + .insert(key.to_string()); } fn fail_sets(&self) { - *self.fail_sets.lock().unwrap() = true; + *self + .fail_sets + .lock() + .expect("memory store fail-sets lock poisoned") = true; } } impl SecretStore for MemorySecretStore { fn get(&self, key: &str) -> Result> { - self.operations.lock().unwrap().push(format!("get:{key}")); - if self.failing_gets.lock().unwrap().contains(key) { + self.operations + .lock() + .expect("memory store operations lock poisoned") + .push(format!("get:{key}")); + if self + .failing_gets + .lock() + .expect("memory store failing-gets lock poisoned") + .contains(key) + { return Err(SlackError::Other(format!("failed get: {key}"))); } - Ok(self.entries.lock().unwrap().get(key).cloned()) + Ok(self + .entries + .lock() + .expect("memory store entries lock poisoned") + .get(key) + .cloned()) } fn set(&self, key: &str, value: &str) -> Result<()> { - self.operations.lock().unwrap().push(format!("set:{key}")); - if *self.fail_sets.lock().unwrap() { + self.operations + .lock() + .expect("memory store operations lock poisoned") + .push(format!("set:{key}")); + if *self + .fail_sets + .lock() + .expect("memory store fail-sets lock poisoned") + { return Err(SlackError::Other("failed set".into())); } self.entries .lock() - .unwrap() + .expect("memory store entries lock poisoned") .insert(key.to_string(), value.to_string()); Ok(()) } @@ -523,9 +556,12 @@ mod tests { fn delete(&self, key: &str) -> Result<()> { self.operations .lock() - .unwrap() + .expect("memory store operations lock poisoned") .push(format!("delete:{key}")); - self.entries.lock().unwrap().remove(key); + self.entries + .lock() + .expect("memory store entries lock poisoned") + .remove(key); Ok(()) } } diff --git a/src/auth/tokens.rs b/src/auth/tokens.rs index 2b93409..811242e 100644 --- a/src/auth/tokens.rs +++ b/src/auth/tokens.rs @@ -345,7 +345,7 @@ mod tests { #[test] fn test_token_type_from_prefix_user() { assert_eq!( - TokenType::from_prefix("xoxp-123456789-0"), + TokenType::from_prefix("xoxp-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic prefix fixture Some(TokenType::UserOAuth) ); } @@ -353,7 +353,7 @@ mod tests { #[test] fn test_token_type_from_prefix_bot() { assert_eq!( - TokenType::from_prefix("xoxb-123456789-0"), + TokenType::from_prefix("xoxb-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic prefix fixture Some(TokenType::BotOAuth) ); } @@ -361,7 +361,7 @@ mod tests { #[test] fn test_token_type_from_prefix_browser() { assert_eq!( - TokenType::from_prefix("xoxc-123456789-0"), + TokenType::from_prefix("xoxc-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic prefix fixture Some(TokenType::Browser) ); } @@ -369,7 +369,10 @@ mod tests { #[test] fn test_token_type_from_prefix_invalid() { assert_eq!(TokenType::from_prefix("invalid-token"), None); - assert_eq!(TokenType::from_prefix("xoxa-123456789-0"), None); + assert_eq!( + TokenType::from_prefix("xoxa-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic invalid-prefix fixture + None + ); assert_eq!(TokenType::from_prefix(""), None); } @@ -382,14 +385,14 @@ mod tests { #[test] fn test_validate_token_format_valid() { - assert!(validate_token_format("xoxp-123456789-0123456789-abcdef").is_ok()); - assert!(validate_token_format("xoxb-123456789-0123456789-abcdef").is_ok()); + assert!(validate_token_format("xoxp-123456789-0123456789-abcdef").is_ok()); // aislop-ignore-line security/hardcoded-secret -- synthetic validation fixture + assert!(validate_token_format("xoxb-123456789-0123456789-abcdef").is_ok()); // aislop-ignore-line security/hardcoded-secret -- synthetic validation fixture assert!(validate_token_format("xoxc-123456789-0123456789-abcdef").is_ok()); } #[test] fn test_validate_token_format_too_short() { - let result = validate_token_format("xoxp-123"); + let result = validate_token_format("xoxp-123"); // aislop-ignore-line security/hardcoded-secret -- synthetic short-token fixture assert!(result.is_err()); assert!(matches!(result.unwrap_err(), SlackError::InvalidToken(_))); } @@ -402,7 +405,7 @@ mod tests { #[test] fn test_validate_token_format_invalid_chars() { - let result = validate_token_format("xoxp-123456789!@#$%"); + let result = validate_token_format("xoxp-123456789!@#$%"); // aislop-ignore-line security/hardcoded-secret -- synthetic malformed fixture assert!(result.is_err()); } @@ -427,7 +430,7 @@ mod tests { #[test] fn test_token_set_new_oauth_user() { let result = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), + "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture "T12345".into(), "Test Workspace".into(), "U12345".into(), @@ -442,7 +445,7 @@ mod tests { #[test] fn test_token_set_new_oauth_bot() { let result = TokenSet::new_oauth( - "xoxb-123456789-0123456789-abcdef".into(), + "xoxb-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture "T12345".into(), "Test Workspace".into(), "U12345".into(), @@ -483,7 +486,7 @@ mod tests { #[test] fn test_token_set_new_browser_rejects_non_xoxc() { let result = TokenSet::new_browser( - "xoxp-123456789-0123456789-abcdef".into(), + "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic invalid-type fixture "xoxd-cookie-value".into(), "T12345".into(), "Test Workspace".into(), @@ -495,7 +498,7 @@ mod tests { #[test] fn test_token_set_validate() { let token_set = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), + "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture "T12345".into(), "Test Workspace".into(), "U12345".into(), @@ -508,7 +511,7 @@ mod tests { #[test] fn test_token_set_supports_search() { let user_token = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), + "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture "T12345".into(), "Test".into(), "U12345".into(), @@ -518,7 +521,7 @@ mod tests { assert!(user_token.supports_search()); let bot_token = TokenSet::new_oauth( - "xoxb-123456789-0123456789-abcdef".into(), + "xoxb-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture "T12345".into(), "Test".into(), "U12345".into(), @@ -541,7 +544,7 @@ mod tests { #[test] fn test_token_set_auth_header() { let token_set = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), + "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture "T12345".into(), "Test".into(), "U12345".into(), @@ -550,14 +553,14 @@ mod tests { .unwrap(); assert_eq!( token_set.auth_header(), - "Bearer xoxp-123456789-0123456789-abcdef" + "Bearer xoxp-123456789-0123456789-abcdef" // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture ); } #[test] fn test_token_set_serialization_roundtrip() { let original = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), + "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture "T12345".into(), "Test Workspace".into(), "U12345".into(), diff --git a/src/cli/auth.rs b/src/cli/auth.rs index 777f7c2..60958a2 100644 --- a/src/cli/auth.rs +++ b/src/cli/auth.rs @@ -834,8 +834,14 @@ mod tests { #[test] fn test_parse_auth_add_token() { - let cli = - Cli::try_parse_from(["slack", "auth", "add", "--token", "xoxp-123456789"]).unwrap(); + let cli = Cli::try_parse_from([ + "slack", + "auth", + "add", + "--token", + "xoxp-123456789", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + ]) + .unwrap(); if let crate::cli::Commands::Auth(auth_cmd) = cli.command { if let AuthCommands::Add { token, @@ -845,7 +851,7 @@ mod tests { .. } = auth_cmd.command { - assert_eq!(token, Some("xoxp-123456789".to_string())); + assert_eq!(token, Some("xoxp-123456789".to_string())); // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture assert!(xoxc.is_none()); assert!(xoxd.is_none()); assert!(!oauth); @@ -910,16 +916,20 @@ mod tests { #[test] fn test_parse_auth_add_conflicts_token_and_oauth() { - let result = - Cli::try_parse_from(["slack", "auth", "add", "--token", "xoxp-123", "--oauth"]); + let result = Cli::try_parse_from([ + "slack", "auth", "add", "--token", + "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + "--oauth", + ]); assert!(result.is_err()); } #[test] fn test_parse_auth_add_conflicts_token_and_xoxc() { let result = Cli::try_parse_from([ - "slack", "auth", "add", "--token", "xoxp-123", "--xoxc", "xoxc-456", "--xoxd", - "xoxd-789", + "slack", "auth", "add", "--token", + "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + "--xoxc", "xoxc-456", "--xoxd", "xoxd-789", ]); assert!(result.is_err()); } @@ -1027,7 +1037,10 @@ mod tests { #[test] fn test_parse_auth_add_positional_conflicts_with_token() { // A positional workspace and an explicit --token are mutually exclusive. - let result = Cli::try_parse_from(["slack", "auth", "add", "myteam", "--token", "xoxp-123"]); + let result = Cli::try_parse_from([ + "slack", "auth", "add", "myteam", "--token", + "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + ]); assert!(result.is_err()); } @@ -1175,7 +1188,7 @@ mod tests { "add", "--from-browser", "--token", - "xoxp-123", + "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture ]); assert!(result.is_err()); } diff --git a/src/main.rs b/src/main.rs index d9fbc5f..68828d0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,20 +19,16 @@ async fn main() { } async fn run() -> i32 { - // Parse CLI arguments let cli = Cli::parse(); - // Initialize tracing based on verbose flag init_tracing(cli.verbose); - // Determine output mode let output_mode = if cli.plain { OutputMode::Plain } else { OutputMode::from_env() }; - // Run the command let result = run_command(&cli).await; // Handle errors From 21c519b8c80dcd6265659ce66159a487fe052867 Mon Sep 17 00:00:00 2001 From: Chris Raethke Date: Thu, 17 Sep 2026 19:40:54 +1000 Subject: [PATCH 2/3] chore: address review feedback --- .aislop/config.yml | 7 +-- .aislop/rules.yml | 22 ++++------ .aislopignore | 2 + .github/workflows/aislop.yml | 2 +- src/api/client.rs | 6 +-- src/api/edge.rs | 4 +- src/auth/browser.rs | 39 ++++++++--------- src/auth/extract/chromium.rs | 9 ++-- src/auth/oauth.rs | 24 +++++------ src/auth/resolve.rs | 16 +++---- src/auth/storage.rs | 75 ++++++++------------------------- src/auth/tokens.rs | 34 +++++++-------- src/cli/auth.rs | 33 ++++++++++----- src/lib.rs | 8 ++++ tests/fixtures/aislop_tokens.rs | 30 +++++++++++++ 15 files changed, 157 insertions(+), 154 deletions(-) create mode 100644 .aislopignore create mode 100644 tests/fixtures/aislop_tokens.rs diff --git a/.aislop/config.yml b/.aislop/config.yml index dbff164..8d8292c 100644 --- a/.aislop/config.yml +++ b/.aislop/config.yml @@ -1,7 +1,8 @@ version: 1 engines: - format: true - lint: true + # CI already runs rustfmt, clippy, and the compiler matrix. + format: false + lint: false code-quality: true ai-slop: true architecture: true @@ -12,7 +13,7 @@ quality: maxNesting: 5 maxParams: 6 lint: - typecheck: true + typecheck: false expoDoctor: false security: audit: true diff --git a/.aislop/rules.yml b/.aislop/rules.yml index 5e3772e..0a00c0b 100644 --- a/.aislop/rules.yml +++ b/.aislop/rules.yml @@ -1,14 +1,8 @@ -# Architecture rules (BYO) -# Uncomment and customize to enforce your project's conventions. -# -# rules: -# - name: no-axios -# type: forbid_import -# match: "axios" -# severity: error -# -# - name: controller-no-db -# type: forbid_import_from_path -# from: "src/controllers/**" -# forbid: "src/db/**" -# severity: error +# Keep credential storage behind the authentication layer. +# CLI modules should use the auth service rather than importing keyring directly. +rules: + - name: no-keyring-outside-auth + type: forbid_import + from: "src/cli/**" + match: "keyring" + severity: error diff --git a/.aislopignore b/.aislopignore new file mode 100644 index 0000000..db5a715 --- /dev/null +++ b/.aislopignore @@ -0,0 +1,2 @@ +# Synthetic credentials are test-only fixtures included by the unit-test crate. +tests/fixtures/aislop_tokens.rs diff --git a/.github/workflows/aislop.yml b/.github/workflows/aislop.yml index 555d1f0..8e6d91e 100644 --- a/.github/workflows/aislop.yml +++ b/.github/workflows/aislop.yml @@ -12,4 +12,4 @@ jobs: - uses: actions/checkout@v4 - uses: scanaislop/aislop@v1 with: - version: latest + version: 0.16.0 diff --git a/src/api/client.rs b/src/api/client.rs index ce50f4f..db11b73 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -582,7 +582,7 @@ mod tests { match token_type { TokenType::UserOAuth => TokenSet { token_type: TokenType::UserOAuth, - access_token: "xoxp-123456789-0123456789-abcdef".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + access_token: crate::test_fixtures::USER_OAUTH.to_string(), xoxd_cookie: None, team_id: "T12345".to_string(), team_name: "Test".to_string(), @@ -593,7 +593,7 @@ mod tests { }, TokenType::BotOAuth => TokenSet { token_type: TokenType::BotOAuth, - access_token: "xoxb-123456789-0123456789-abcdef".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + access_token: crate::test_fixtures::BOT_OAUTH.to_string(), xoxd_cookie: None, team_id: "T12345".to_string(), team_name: "Test".to_string(), @@ -604,7 +604,7 @@ mod tests { }, TokenType::Browser => TokenSet { token_type: TokenType::Browser, - access_token: "xoxc-123456789-0123456789-abcdef".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic browser fixture + access_token: crate::test_fixtures::BROWSER.to_string(), xoxd_cookie: Some("xoxd-test-cookie".to_string()), team_id: "T12345".to_string(), team_name: "Test".to_string(), diff --git a/src/api/edge.rs b/src/api/edge.rs index 6f0da35..fb0eb38 100644 --- a/src/api/edge.rs +++ b/src/api/edge.rs @@ -323,7 +323,7 @@ mod tests { fn create_browser_token() -> TokenSet { TokenSet { token_type: TokenType::Browser, - access_token: "xoxc-1234567890-0123456789-abcdefghij".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic browser fixture + access_token: crate::test_fixtures::BROWSER.to_string(), xoxd_cookie: Some("xoxd-test-cookie-value".to_string()), team_id: "T12345".to_string(), team_name: "Test Team".to_string(), @@ -337,7 +337,7 @@ mod tests { fn create_oauth_token() -> TokenSet { TokenSet { token_type: TokenType::UserOAuth, - access_token: "xoxp-1234567890-0123456789-abcdefghij".to_string(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + access_token: crate::test_fixtures::USER_OAUTH.to_string(), xoxd_cookie: None, team_id: "T12345".to_string(), team_name: "Test Team".to_string(), diff --git a/src/auth/browser.rs b/src/auth/browser.rs index cc0c826..0497f2c 100644 --- a/src/auth/browser.rs +++ b/src/auth/browser.rs @@ -168,8 +168,8 @@ mod tests { #[test] fn test_browser_tokens_valid() { let tokens = BrowserTokens::new( - "xoxc-1234567890-abcdef123456789012345678901234567890".into(), - "xoxd-abcdefghijklmnopqrstuvwxyz1234567890".into(), + crate::test_fixtures::BROWSER_LONG.into(), + crate::test_fixtures::BROWSER_COOKIE.into(), ); assert!(tokens.validate().is_ok()); @@ -179,8 +179,8 @@ mod tests { #[test] fn test_browser_tokens_invalid_xoxc_prefix() { let tokens = BrowserTokens::new( - "xoxp-1234567890-abcdef123456789012345678901234567890".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic invalid-prefix fixture - "xoxd-abcdefghijklmnopqrstuvwxyz1234567890".into(), + crate::test_fixtures::INVALID_BROWSER_PREFIX.into(), + crate::test_fixtures::BROWSER_COOKIE.into(), ); let result = tokens.validate(); @@ -191,8 +191,8 @@ mod tests { #[test] fn test_browser_tokens_xoxc_too_short() { let tokens = BrowserTokens::new( - "xoxc-123".into(), - "xoxd-abcdefghijklmnopqrstuvwxyz1234567890".into(), + crate::test_fixtures::SHORT_BROWSER.into(), + crate::test_fixtures::BROWSER_COOKIE.into(), ); let result = tokens.validate(); @@ -202,8 +202,8 @@ mod tests { #[test] fn test_browser_tokens_xoxc_invalid_chars() { let tokens = BrowserTokens::new( - "xoxc-1234567890-abcdef!@#$%^&*()".into(), - "xoxd-abcdefghijklmnopqrstuvwxyz1234567890".into(), + crate::test_fixtures::INVALID_BROWSER_CHARS.into(), + crate::test_fixtures::BROWSER_COOKIE.into(), ); let result = tokens.validate(); @@ -212,10 +212,7 @@ mod tests { #[test] fn test_browser_tokens_empty_xoxd() { - let tokens = BrowserTokens::new( - "xoxc-1234567890-abcdef123456789012345678901234567890".into(), - "".into(), - ); + let tokens = BrowserTokens::new(crate::test_fixtures::BROWSER_LONG.into(), "".into()); let result = tokens.validate(); assert!(result.is_err()); @@ -224,8 +221,8 @@ mod tests { #[test] fn test_browser_tokens_xoxd_with_newlines() { let tokens = BrowserTokens::new( - "xoxc-1234567890-abcdef123456789012345678901234567890".into(), - "xoxd-abc\ndef".into(), + crate::test_fixtures::BROWSER_LONG.into(), + crate::test_fixtures::BROWSER_COOKIE_NEWLINE.into(), ); let result = tokens.validate(); @@ -235,8 +232,8 @@ mod tests { #[test] fn test_browser_tokens_xoxd_carriage_return() { let tokens = BrowserTokens::new( - "xoxc-1234567890-abcdef123456789012345678901234567890".into(), - "xoxd-abc\rdef".into(), + crate::test_fixtures::BROWSER_LONG.into(), + crate::test_fixtures::BROWSER_COOKIE_CARRIAGE_RETURN.into(), ); let result = tokens.validate(); @@ -248,8 +245,8 @@ mod tests { // Older xoxd cookies might not start with xoxd- // but should be long encoded values let tokens = BrowserTokens::new( - "xoxc-1234567890-abcdef123456789012345678901234567890".into(), - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic legacy cookie fixture + crate::test_fixtures::BROWSER_LONG.into(), + crate::test_fixtures::LEGACY_BROWSER_COOKIE.into(), ); // Long JWT-like cookie should be accepted @@ -260,7 +257,7 @@ mod tests { fn test_browser_tokens_short_non_xoxd() { // Short non-xoxd cookie is likely wrong let tokens = BrowserTokens::new( - "xoxc-1234567890-abcdef123456789012345678901234567890".into(), + crate::test_fixtures::BROWSER_LONG.into(), "shortcookie".into(), ); @@ -271,8 +268,8 @@ mod tests { #[test] fn test_is_valid_format() { let valid = BrowserTokens::new( - "xoxc-1234567890-abcdef123456789012345678901234567890".into(), - "xoxd-abcdefghijklmnopqrstuvwxyz1234567890".into(), + crate::test_fixtures::BROWSER_LONG.into(), + crate::test_fixtures::BROWSER_COOKIE.into(), ); assert!(valid.is_valid_format()); diff --git a/src/auth/extract/chromium.rs b/src/auth/extract/chromium.rs index cc7dacf..9b0d962 100644 --- a/src/auth/extract/chromium.rs +++ b/src/auth/extract/chromium.rs @@ -857,10 +857,11 @@ mod tests { fn strict_and_fallback_parsers_cover_partial_records() { assert!(parse_local_config("localConfig_v2 no object").is_empty()); assert!(parse_local_config("localConfig_v2{broken}").is_empty()); - assert!(parse_local_config( - "localConfig_v2{\"teams\":{\"T1234567\":{\"token\":\"xoxp-not-client\"},\"T7654321\":{}}}}" // aislop-ignore-line security/hardcoded-secret -- synthetic parser fixture - ) - .is_empty()); + let non_client = format!( + "localConfig_v2{{\"teams\":{{\"T1234567\":{{\"token\":\"{}\"}},\"T7654321\":{{}}}}}}", + crate::test_fixtures::PARSER_NON_CLIENT + ); + assert!(parse_local_config(&non_client).is_empty()); let bare = "localConfig_v2{\"workspace\":{\"id\":\"TEXPLICIT\",\"token\":\"xoxc-bare-map-123456\"}}"; let tokens = parse_local_config(bare); diff --git a/src/auth/oauth.rs b/src/auth/oauth.rs index 6753068..aba0e5a 100644 --- a/src/auth/oauth.rs +++ b/src/auth/oauth.rs @@ -595,7 +595,7 @@ mod tests { fn test_parse_oauth_response_success_bot_token() { let body: serde_json::Value = serde_json::json!({ "ok": true, - "access_token": "xoxb-123456789-0123456789-abcdefghijklmnop", // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture + "access_token": crate::test_fixtures::OAUTH_BOT_RESPONSE, "token_type": "bot", "scope": "channels:read,users:read", "bot_user_id": "UBOT12345", @@ -611,8 +611,7 @@ mod tests { let token_set = result.unwrap(); assert_eq!( token_set.access_token, - // aislop-ignore-next-line security/hardcoded-secret -- synthetic OAuth response fixture - "xoxb-123456789-0123456789-abcdefghijklmnop" + crate::test_fixtures::OAUTH_BOT_RESPONSE ); assert_eq!(token_set.team_id, "T12345678"); assert_eq!(token_set.team_name, "Test Workspace"); @@ -625,12 +624,12 @@ mod tests { fn test_parse_oauth_response_success_user_token() { let body: serde_json::Value = serde_json::json!({ "ok": true, - "access_token": "xoxp-123456789-0123456789-0123456789-abcdef", // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture + "access_token": crate::test_fixtures::OAUTH_USER_RESPONSE, "token_type": "user", "scope": "channels:read,chat:write", "authed_user": { "id": "U12345678", - "access_token": "xoxp-123456789-0123456789-0123456789-abcdef" // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture + "access_token": crate::test_fixtures::OAUTH_USER_RESPONSE }, "team": { "id": "T98765432", @@ -644,8 +643,7 @@ mod tests { let token_set = result.unwrap(); assert_eq!( token_set.access_token, - // aislop-ignore-next-line security/hardcoded-secret -- synthetic OAuth response fixture - "xoxp-123456789-0123456789-0123456789-abcdef" + crate::test_fixtures::OAUTH_USER_RESPONSE ); assert_eq!(token_set.team_id, "T98765432"); assert_eq!(token_set.team_name, "User Workspace"); @@ -681,7 +679,7 @@ mod tests { "scope": "search:read", "authed_user": { "id": "UUSER1234", - "access_token": "xoxp-user-token-here-abcdef123" // aislop-ignore-line security/hardcoded-secret -- synthetic fallback fixture + "access_token": crate::test_fixtures::OAUTH_USER_FALLBACK }, "team": { "id": "TUSER1234", @@ -695,7 +693,7 @@ mod tests { let token_set = result.unwrap(); assert_eq!( token_set.access_token, - "xoxp-user-token-here-abcdef123" // aislop-ignore-line security/hardcoded-secret -- synthetic fallback fixture + crate::test_fixtures::OAUTH_USER_FALLBACK ); assert_eq!(token_set.user_id, "UUSER1234"); assert_eq!(token_set.token_type, crate::auth::TokenType::UserOAuth); @@ -706,7 +704,7 @@ mod tests { // Test case where neither user_id nor bot_user_id is present let body: serde_json::Value = serde_json::json!({ "ok": true, - "access_token": "xoxb-minimal-token-here-1234", // aislop-ignore-line security/hardcoded-secret -- synthetic fallback fixture + "access_token": crate::test_fixtures::OAUTH_BOT_FALLBACK, "token_type": "bot", "scope": "", "team": { @@ -756,7 +754,7 @@ mod tests { #[test] fn exchange_code_posts_form_and_maps_success() { use mockito::Matcher; - let expected_token = "xoxb-test-access-token"; + let expected_token = crate::test_fixtures::OAUTH_BOT_EXCHANGE; let mut server = mockito::Server::new(); let mock = server @@ -923,7 +921,7 @@ mod tests { fn oauth_success_mock(server: &mut mockito::Server, expected_code: &str) -> mockito::Mock { use mockito::Matcher; - let expected_token = "xoxp-manual-access-token"; // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture + let expected_token = crate::test_fixtures::OAUTH_MANUAL; server .mock("POST", "/oauth.v2.access") @@ -962,7 +960,7 @@ mod tests { .unwrap(); mock.assert(); - assert_eq!(token.access_token, "xoxp-manual-access-token"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth response fixture + assert_eq!(token.access_token, crate::test_fixtures::OAUTH_MANUAL); assert!(presented_url.unwrap().contains("state=known-state")); } diff --git a/src/auth/resolve.rs b/src/auth/resolve.rs index af7c341..1ccfcd3 100644 --- a/src/auth/resolve.rs +++ b/src/auth/resolve.rs @@ -64,9 +64,9 @@ mod tests { #[test] fn test_override_user_oauth_token() { - let token = resolve_token(None, Some("xoxp-1234567890-abcdef")).unwrap(); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + let token = resolve_token(None, Some(crate::test_fixtures::USER_OVERRIDE)).unwrap(); assert_eq!(token.token_type, TokenType::UserOAuth); - assert_eq!(token.access_token, "xoxp-1234567890-abcdef"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + assert_eq!(token.access_token, crate::test_fixtures::USER_OVERRIDE); assert_eq!(token.team_id, "unknown"); assert_eq!(token.team_name, "unknown"); assert_eq!(token.user_id, "unknown"); @@ -76,9 +76,9 @@ mod tests { #[test] fn test_override_bot_oauth_token() { - let token = resolve_token(None, Some("xoxb-1234567890-abcdef")).unwrap(); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + let token = resolve_token(None, Some(crate::test_fixtures::BOT_OVERRIDE)).unwrap(); assert_eq!(token.token_type, TokenType::BotOAuth); - assert_eq!(token.access_token, "xoxb-1234567890-abcdef"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + assert_eq!(token.access_token, crate::test_fixtures::BOT_OVERRIDE); } #[test] @@ -87,15 +87,15 @@ mod tests { // never consulted (a nonexistent workspace would otherwise error). let token = resolve_token( Some("definitely-not-a-real-workspace"), - Some("xoxp-1234567890-abcdef"), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + Some(crate::test_fixtures::USER_OVERRIDE), ) .unwrap(); - assert_eq!(token.access_token, "xoxp-1234567890-abcdef"); // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + assert_eq!(token.access_token, crate::test_fixtures::USER_OVERRIDE); } #[test] fn test_override_browser_token_rejected() { - let err = resolve_token(None, Some("xoxc-1234567890-abcdef")).unwrap_err(); // aislop-ignore-line security/hardcoded-secret -- synthetic browser fixture + let err = resolve_token(None, Some(crate::test_fixtures::BROWSER_OVERRIDE)).unwrap_err(); match err { SlackError::InvalidToken(msg) => { assert!(msg.contains("--xoxc and --xoxd"), "unexpected msg: {}", msg); @@ -122,7 +122,7 @@ mod tests { #[test] fn test_override_malformed_token_rejected() { // Valid prefix but invalid characters fails TokenSet validation. - let err = resolve_token(None, Some("xoxp-bad token!")).unwrap_err(); // aislop-ignore-line security/hardcoded-secret -- synthetic malformed fixture + let err = resolve_token(None, Some(crate::test_fixtures::MALFORMED_OVERRIDE)).unwrap_err(); assert!(matches!(err, SlackError::InvalidToken(_))); } } diff --git a/src/auth/storage.rs b/src/auth/storage.rs index 0e881df..27ef6e7 100644 --- a/src/auth/storage.rs +++ b/src/auth/storage.rs @@ -475,93 +475,52 @@ mod tests { } impl MemorySecretStore { + fn lock(mutex: &Mutex) -> std::sync::MutexGuard<'_, T> { + mutex.lock().expect("memory store lock poisoned") + } + fn seed(&self, key: &str, value: impl Into) { - self.entries - .lock() - .expect("memory store entries lock poisoned") - .insert(key.to_string(), value.into()); + Self::lock(&self.entries).insert(key.to_string(), value.into()); } fn value(&self, key: &str) -> Option { - self.entries - .lock() - .expect("memory store entries lock poisoned") - .get(key) - .cloned() + Self::lock(&self.entries).get(key).cloned() } fn operations(&self) -> Vec { - self.operations - .lock() - .expect("memory store operations lock poisoned") - .clone() + Self::lock(&self.operations).clone() } fn fail_get(&self, key: &str) { - self.failing_gets - .lock() - .expect("memory store failing-gets lock poisoned") - .insert(key.to_string()); + Self::lock(&self.failing_gets).insert(key.to_string()); } fn fail_sets(&self) { - *self - .fail_sets - .lock() - .expect("memory store fail-sets lock poisoned") = true; + *Self::lock(&self.fail_sets) = true; } } impl SecretStore for MemorySecretStore { fn get(&self, key: &str) -> Result> { - self.operations - .lock() - .expect("memory store operations lock poisoned") - .push(format!("get:{key}")); - if self - .failing_gets - .lock() - .expect("memory store failing-gets lock poisoned") - .contains(key) - { + Self::lock(&self.operations).push(format!("get:{key}")); + if Self::lock(&self.failing_gets).contains(key) { return Err(SlackError::Other(format!("failed get: {key}"))); } - Ok(self - .entries - .lock() - .expect("memory store entries lock poisoned") - .get(key) - .cloned()) + Ok(Self::lock(&self.entries).get(key).cloned()) } fn set(&self, key: &str, value: &str) -> Result<()> { - self.operations - .lock() - .expect("memory store operations lock poisoned") - .push(format!("set:{key}")); - if *self - .fail_sets - .lock() - .expect("memory store fail-sets lock poisoned") - { + Self::lock(&self.operations).push(format!("set:{key}")); + if *Self::lock(&self.fail_sets) { return Err(SlackError::Other("failed set".into())); } - self.entries - .lock() - .expect("memory store entries lock poisoned") - .insert(key.to_string(), value.to_string()); + Self::lock(&self.entries).insert(key.to_string(), value.to_string()); Ok(()) } fn delete(&self, key: &str) -> Result<()> { - self.operations - .lock() - .expect("memory store operations lock poisoned") - .push(format!("delete:{key}")); - self.entries - .lock() - .expect("memory store entries lock poisoned") - .remove(key); + Self::lock(&self.operations).push(format!("delete:{key}")); + Self::lock(&self.entries).remove(key); Ok(()) } } diff --git a/src/auth/tokens.rs b/src/auth/tokens.rs index 811242e..a212cd9 100644 --- a/src/auth/tokens.rs +++ b/src/auth/tokens.rs @@ -345,7 +345,7 @@ mod tests { #[test] fn test_token_type_from_prefix_user() { assert_eq!( - TokenType::from_prefix("xoxp-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic prefix fixture + TokenType::from_prefix(crate::test_fixtures::SHORT_PREFIX), Some(TokenType::UserOAuth) ); } @@ -353,7 +353,7 @@ mod tests { #[test] fn test_token_type_from_prefix_bot() { assert_eq!( - TokenType::from_prefix("xoxb-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic prefix fixture + TokenType::from_prefix(crate::test_fixtures::BOT_PREFIX), Some(TokenType::BotOAuth) ); } @@ -361,7 +361,7 @@ mod tests { #[test] fn test_token_type_from_prefix_browser() { assert_eq!( - TokenType::from_prefix("xoxc-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic prefix fixture + TokenType::from_prefix(crate::test_fixtures::BROWSER_PREFIX), Some(TokenType::Browser) ); } @@ -370,7 +370,7 @@ mod tests { fn test_token_type_from_prefix_invalid() { assert_eq!(TokenType::from_prefix("invalid-token"), None); assert_eq!( - TokenType::from_prefix("xoxa-123456789-0"), // aislop-ignore-line security/hardcoded-secret -- synthetic invalid-prefix fixture + TokenType::from_prefix(crate::test_fixtures::INVALID_PREFIX), None ); assert_eq!(TokenType::from_prefix(""), None); @@ -385,14 +385,14 @@ mod tests { #[test] fn test_validate_token_format_valid() { - assert!(validate_token_format("xoxp-123456789-0123456789-abcdef").is_ok()); // aislop-ignore-line security/hardcoded-secret -- synthetic validation fixture - assert!(validate_token_format("xoxb-123456789-0123456789-abcdef").is_ok()); // aislop-ignore-line security/hardcoded-secret -- synthetic validation fixture + assert!(validate_token_format(crate::test_fixtures::USER_OAUTH).is_ok()); + assert!(validate_token_format(crate::test_fixtures::BOT_OAUTH).is_ok()); assert!(validate_token_format("xoxc-123456789-0123456789-abcdef").is_ok()); } #[test] fn test_validate_token_format_too_short() { - let result = validate_token_format("xoxp-123"); // aislop-ignore-line security/hardcoded-secret -- synthetic short-token fixture + let result = validate_token_format(crate::test_fixtures::SHORT_TOKEN); assert!(result.is_err()); assert!(matches!(result.unwrap_err(), SlackError::InvalidToken(_))); } @@ -405,7 +405,7 @@ mod tests { #[test] fn test_validate_token_format_invalid_chars() { - let result = validate_token_format("xoxp-123456789!@#$%"); // aislop-ignore-line security/hardcoded-secret -- synthetic malformed fixture + let result = validate_token_format(crate::test_fixtures::INVALID_CHARS); assert!(result.is_err()); } @@ -430,7 +430,7 @@ mod tests { #[test] fn test_token_set_new_oauth_user() { let result = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + crate::test_fixtures::USER_OAUTH.into(), "T12345".into(), "Test Workspace".into(), "U12345".into(), @@ -445,7 +445,7 @@ mod tests { #[test] fn test_token_set_new_oauth_bot() { let result = TokenSet::new_oauth( - "xoxb-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + crate::test_fixtures::BOT_OAUTH.into(), "T12345".into(), "Test Workspace".into(), "U12345".into(), @@ -486,7 +486,7 @@ mod tests { #[test] fn test_token_set_new_browser_rejects_non_xoxc() { let result = TokenSet::new_browser( - "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic invalid-type fixture + crate::test_fixtures::USER_OAUTH.into(), "xoxd-cookie-value".into(), "T12345".into(), "Test Workspace".into(), @@ -498,7 +498,7 @@ mod tests { #[test] fn test_token_set_validate() { let token_set = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + crate::test_fixtures::USER_OAUTH.into(), "T12345".into(), "Test Workspace".into(), "U12345".into(), @@ -511,7 +511,7 @@ mod tests { #[test] fn test_token_set_supports_search() { let user_token = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + crate::test_fixtures::USER_OAUTH.into(), "T12345".into(), "Test".into(), "U12345".into(), @@ -521,7 +521,7 @@ mod tests { assert!(user_token.supports_search()); let bot_token = TokenSet::new_oauth( - "xoxb-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + crate::test_fixtures::BOT_OAUTH.into(), "T12345".into(), "Test".into(), "U12345".into(), @@ -544,7 +544,7 @@ mod tests { #[test] fn test_token_set_auth_header() { let token_set = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + crate::test_fixtures::USER_OAUTH.into(), "T12345".into(), "Test".into(), "U12345".into(), @@ -553,14 +553,14 @@ mod tests { .unwrap(); assert_eq!( token_set.auth_header(), - "Bearer xoxp-123456789-0123456789-abcdef" // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + format!("Bearer {}", crate::test_fixtures::USER_OAUTH) ); } #[test] fn test_token_set_serialization_roundtrip() { let original = TokenSet::new_oauth( - "xoxp-123456789-0123456789-abcdef".into(), // aislop-ignore-line security/hardcoded-secret -- synthetic OAuth fixture + crate::test_fixtures::USER_OAUTH.into(), "T12345".into(), "Test Workspace".into(), "U12345".into(), diff --git a/src/cli/auth.rs b/src/cli/auth.rs index 60958a2..8f9dbe6 100644 --- a/src/cli/auth.rs +++ b/src/cli/auth.rs @@ -839,7 +839,7 @@ mod tests { "auth", "add", "--token", - "xoxp-123456789", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + crate::test_fixtures::CLI_TOKEN, ]) .unwrap(); if let crate::cli::Commands::Auth(auth_cmd) = cli.command { @@ -851,7 +851,7 @@ mod tests { .. } = auth_cmd.command { - assert_eq!(token, Some("xoxp-123456789".to_string())); // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + assert_eq!(token, Some(crate::test_fixtures::CLI_TOKEN.to_string())); assert!(xoxc.is_none()); assert!(xoxd.is_none()); assert!(!oauth); @@ -917,8 +917,11 @@ mod tests { #[test] fn test_parse_auth_add_conflicts_token_and_oauth() { let result = Cli::try_parse_from([ - "slack", "auth", "add", "--token", - "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + "slack", + "auth", + "add", + "--token", + crate::test_fixtures::CLI_SHORT_TOKEN, "--oauth", ]); assert!(result.is_err()); @@ -927,9 +930,15 @@ mod tests { #[test] fn test_parse_auth_add_conflicts_token_and_xoxc() { let result = Cli::try_parse_from([ - "slack", "auth", "add", "--token", - "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture - "--xoxc", "xoxc-456", "--xoxd", "xoxd-789", + "slack", + "auth", + "add", + "--token", + crate::test_fixtures::CLI_SHORT_TOKEN, + "--xoxc", + "xoxc-456", + "--xoxd", + "xoxd-789", ]); assert!(result.is_err()); } @@ -1038,8 +1047,12 @@ mod tests { fn test_parse_auth_add_positional_conflicts_with_token() { // A positional workspace and an explicit --token are mutually exclusive. let result = Cli::try_parse_from([ - "slack", "auth", "add", "myteam", "--token", - "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + "slack", + "auth", + "add", + "myteam", + "--token", + crate::test_fixtures::CLI_SHORT_TOKEN, ]); assert!(result.is_err()); } @@ -1188,7 +1201,7 @@ mod tests { "add", "--from-browser", "--token", - "xoxp-123", // aislop-ignore-line security/hardcoded-secret -- synthetic CLI fixture + crate::test_fixtures::CLI_SHORT_TOKEN, ]); assert!(result.is_err()); } diff --git a/src/lib.rs b/src/lib.rs index f51e010..e020667 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,3 +5,11 @@ pub mod error; pub mod models; pub mod output; pub mod utils; + +#[cfg(test)] +pub(crate) mod test_fixtures { + include!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/aislop_tokens.rs" + )); +} diff --git a/tests/fixtures/aislop_tokens.rs b/tests/fixtures/aislop_tokens.rs new file mode 100644 index 0000000..1463878 --- /dev/null +++ b/tests/fixtures/aislop_tokens.rs @@ -0,0 +1,30 @@ +pub const USER_OAUTH: &str = "xoxp-123456789-0123456789-abcdef"; +pub const BOT_OAUTH: &str = "xoxb-123456789-0123456789-abcdef"; +pub const BROWSER: &str = "xoxc-123456789-0123456789-abcdef"; +pub const BROWSER_LONG: &str = "xoxc-1234567890-abcdef123456789012345678901234567890"; +pub const INVALID_BROWSER_PREFIX: &str = "xoxp-1234567890-abcdef123456789012345678901234567890"; +pub const SHORT_BROWSER: &str = "xoxc-123"; +pub const INVALID_BROWSER_CHARS: &str = "xoxc-1234567890-abcdef!@#$%^&*()"; +pub const BROWSER_COOKIE: &str = "xoxd-abcdefghijklmnopqrstuvwxyz1234567890"; +pub const BROWSER_COOKIE_NEWLINE: &str = "xoxd-abc\ndef"; +pub const BROWSER_COOKIE_CARRIAGE_RETURN: &str = "xoxd-abc\rdef"; +pub const LEGACY_BROWSER_COOKIE: &str = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; +pub const PARSER_NON_CLIENT: &str = "xoxp-not-client"; +pub const OAUTH_USER_RESPONSE: &str = "xoxp-123456789-0123456789-0123456789-abcdef"; +pub const OAUTH_BOT_RESPONSE: &str = "xoxb-123456789-0123456789-abcdefghijklmnop"; +pub const OAUTH_USER_FALLBACK: &str = "xoxp-user-token-here-abcdef123"; +pub const OAUTH_BOT_FALLBACK: &str = "xoxb-minimal-token-here-1234"; +pub const OAUTH_BOT_EXCHANGE: &str = "xoxb-test-access-token"; +pub const OAUTH_MANUAL: &str = "xoxp-manual-access-token"; +pub const USER_OVERRIDE: &str = "xoxp-1234567890-abcdef"; +pub const BOT_OVERRIDE: &str = "xoxb-1234567890-abcdef"; +pub const BROWSER_OVERRIDE: &str = "xoxc-1234567890-abcdef"; +pub const MALFORMED_OVERRIDE: &str = "xoxp-bad token!"; +pub const SHORT_PREFIX: &str = "xoxp-123456789-0"; +pub const BOT_PREFIX: &str = "xoxb-123456789-0"; +pub const BROWSER_PREFIX: &str = "xoxc-123456789-0"; +pub const INVALID_PREFIX: &str = "xoxa-123456789-0"; +pub const SHORT_TOKEN: &str = "xoxp-123"; +pub const INVALID_CHARS: &str = "xoxp-123456789!@#$%"; +pub const CLI_TOKEN: &str = "xoxp-123456789"; +pub const CLI_SHORT_TOKEN: &str = "xoxp-123"; From c67c74c2b7e8e48b50520684116cf44db7f40df9 Mon Sep 17 00:00:00 2001 From: Chris Raethke Date: Thu, 17 Sep 2026 19:44:41 +1000 Subject: [PATCH 3/3] chore: pin Aislop 0.16.1 --- .github/workflows/aislop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/aislop.yml b/.github/workflows/aislop.yml index 8e6d91e..1a298f2 100644 --- a/.github/workflows/aislop.yml +++ b/.github/workflows/aislop.yml @@ -12,4 +12,4 @@ jobs: - uses: actions/checkout@v4 - uses: scanaislop/aislop@v1 with: - version: 0.16.0 + version: 0.16.1