Skip to content

feat: support Mac App Store version of Slack#1

Open
brandon-dacrib wants to merge 1 commit intotherohitdas:mainfrom
brandon-dacrib:main
Open

feat: support Mac App Store version of Slack#1
brandon-dacrib wants to merge 1 commit intotherohitdas:mainfrom
brandon-dacrib:main

Conversation

@brandon-dacrib
Copy link
Copy Markdown

@brandon-dacrib brandon-dacrib commented Feb 4, 2026

Problem

slk doesn't work with the Mac App Store version of Slack.

When running slk auth with the App Store version installed, the command hangs indefinitely or fails silently. This happens because the keychain query is ambiguous.

Root cause

The Mac App Store version stores its encryption key under a different keychain account name:

Slack Version Keychain Service Keychain Account
Direct download (slack.com) Slack Safe Storage Slack
Mac App Store Slack Safe Storage Slack App Store Key

The current code runs:

security find-generic-password -s "Slack Safe Storage" -w

Without the -a (account) flag, this query is ambiguous when both keychain entries exist, or returns the wrong key when only the App Store entry exists. The security command may:

  • Hang waiting for user disambiguation
  • Return the wrong key (causing decryption to fail)
  • Prompt repeatedly for keychain access

Additional issue: sandboxed data path

The App Store version also stores its data in a sandboxed container:

~/Library/Containers/com.tinyspeck.slackmacgap/Data/Library/Application Support/Slack/

Instead of the expected:

~/Library/Application Support/Slack/

Users need to create a symlink for slk to find the cookies and LevelDB files.

Solution

Updated getKeychainKey() to explicitly specify the account name:

// Try Mac App Store account first, then fall back to direct-download
try {
  execSync('security find-generic-password -s "Slack Safe Storage" -a "Slack App Store Key" -w', ...)
} catch {
  execSync('security find-generic-password -s "Slack Safe Storage" -a "Slack" -w', ...)
}

This makes the keychain query unambiguous and works with both Slack versions.

Changes

  1. src/auth.js — Updated getKeychainKey() to try App Store account first, fall back to direct-download
  2. README.md — Added documentation for both Slack versions and symlink instructions

Testing

Tested on macOS Sequoia with the Mac App Store version of Slack:

  1. Created symlink: ln -s ~/Library/Containers/com.tinyspeck.slackmacgap/.../Slack ~/Library/Application\ Support/Slack
  2. Ran slk auth — successfully authenticated

Also verified the fallback works by testing with the direct-download version's keychain entry.

The Mac App Store version of Slack uses a different keychain account name
('Slack App Store Key') compared to the direct-download version ('Slack').
Both use the same service name 'Slack Safe Storage'.

This change tries the App Store account first, then falls back to the
direct-download account, making slk work with both installation methods.

Also updated README to document both versions and the symlink workaround
for App Store users (data lives in a sandboxed container path).
@therohitdas
Copy link
Copy Markdown
Owner

Thanks, I will look into this!

@therohitdas
Copy link
Copy Markdown
Owner

@claude Check this pr for me

@claude
Copy link
Copy Markdown

claude bot commented Feb 7, 2026

Claude finished @therohitdas's task in 1m 43s —— View job


Code Review

Reviewing PR #1: feat: support Mac App Store version of Slack


Summary

This PR adds support for the Mac App Store version of Slack by handling the different keychain account name. The approach is solid and the problem is well-documented. However, there are several issues that need to be addressed before merging.

🔴 Critical Issues

1. Silent Error Swallowing in Try-Catch (src/auth.js:25-31)

The proposed try-catch pattern silently swallows errors, which will make debugging extremely difficult:

try {
  execSync('security find-generic-password -s "Slack Safe Storage" -a "Slack App Store Key" -w', ...)
} catch {
  execSync('security find-generic-password -s "Slack Safe Storage" -a "Slack" -w', ...)
}

Problems:

  • If both keychain queries fail, the user gets a generic error from the second execSync without knowing that the App Store version was tried first
  • The actual error (keychain access denied, key not found, etc.) is hidden
  • Users can't distinguish between "neither version installed" vs "keychain access denied" vs "other error"

Recommendation:
Distinguish between "not found" errors and other errors. Only fall back if the App Store key is genuinely not found, not for permission errors or other issues.

function getKeychainKey() {
  // Try Mac App Store version first
  try {
    return Buffer.from(
      execSync('security find-generic-password -s "Slack Safe Storage" -a "Slack App Store Key" -w', {
        encoding: "utf-8",
        stdio: ['pipe', 'pipe', 'pipe']
      }).trim()
    );
  } catch (err) {
    // Only fallback if the key wasn't found (not for permission errors, etc.)
    if (!err.message.includes('could not be found')) {
      throw new Error(`Keychain access failed: ${err.message}`);
    }
  }
  
  // Fall back to direct-download version
  try {
    return Buffer.from(
      execSync('security find-generic-password -s "Slack Safe Storage" -a "Slack" -w', {
        encoding: "utf-8",
        stdio: ['pipe', 'pipe', 'pipe']
      }).trim()
    );
  } catch (err) {
    throw new Error(
      'Could not find Slack keychain entry. Make sure Slack desktop app is installed and logged in.\n' +
      'Tried both Mac App Store ("Slack App Store Key") and direct download ("Slack") versions.'
    );
  }
}

Fix error handling →

2. Incomplete Documentation - Symlink Required (README.md)

The PR description mentions that App Store users need to create a symlink, but I don't see this critical information in the README changes. This is a blocking requirement for App Store users.

Missing from README:

# Required for Mac App Store version:
ln -s ~/Library/Containers/com.tinyspeck.slackmacgap/Data/Library/Application\ Support/Slack \
      ~/Library/Application\ Support/Slack

Recommendation: Add a dedicated subsection under "Authentication" that explains:

  1. Why the symlink is needed (sandboxed container path)
  2. How to create it
  3. How to check which version you have installed

Add symlink docs →

⚠️ Medium Issues

3. Order Assumption May Break for Some Users (src/auth.js:26-30)

Trying App Store first assumes most users will have the App Store version, but:

  • The direct-download version is still very common (especially among developers)
  • Trying App Store first adds unnecessary latency for direct-download users
  • Both versions can coexist with both keychain entries

Consideration: Is there a way to detect which version is actually running? Check for:

  • /Applications/Slack.app/Contents/MacOS/Slack (direct download)
  • /Applications/Slack.app with App Store bundle ID (App Store)

Or just document that App Store users get priority (it's a minor perf hit).

4. Validation Testing (Testing Section)

The PR description says "Also verified the fallback works by testing with the direct-download version's keychain entry" - but this isn't clear:

  • Did you test with BOTH entries present?
  • Did you test with ONLY direct-download entry?
  • Did you test the error path when neither is present?

💡 Minor Suggestions

5. Code Comment Clarity

Consider adding a comment explaining why App Store is tried first:

function getKeychainKey() {
  // Try Mac App Store account first (more common in recent installs)
  // Account name: "Slack App Store Key"
  try {
    // ...

6. Consistent Error Messages

The error message pattern should match the rest of the codebase. Check how other auth errors are formatted.

🔐 Security Review

No security issues identified:

  • Still using the same keychain query mechanism
  • No new attack surface introduced
  • Fallback behavior is safe (tries App Store, then direct-download)

✅ What's Good

  1. Problem Analysis - Excellent documentation of the root cause with table showing both versions
  2. Fallback Strategy - Smart to try both keychain entries rather than requiring config
  3. Testing - Good that you tested on macOS Sequoia with the App Store version
  4. Zero Breaking Changes - Existing users with direct-download version are unaffected

📋 Before Merging

  • Fix error handling to distinguish "not found" vs "permission denied"
  • Add Mac App Store symlink documentation to README.md
  • Test with both keychain entries present
  • Test with only direct-download entry
  • Test error path when neither entry exists
  • Consider adding detection of which Slack version is installed (optional)

Overall Assessment: This is a valuable feature that solves a real problem, but needs better error handling and documentation before merging. The core approach is sound.


therohitdas pushed a commit that referenced this pull request Feb 7, 2026
Instead of requiring App Store users to manually create a symlink,
auto-detect the Slack data directory at runtime by checking both the
direct-download and App Store sandbox paths. Also handle both keychain
account names ("Slack" vs "Slack App Store Key") with smart ordering
based on which variant was detected.

Addresses PR #1 feedback.

https://claude.ai/code/session_01TPCTdT8Q7mKBW499t4DPJY
therohitdas pushed a commit that referenced this pull request Feb 7, 2026
Instead of requiring App Store users to manually create a symlink,
auto-detect the Slack data directory at runtime by checking both the
direct-download and App Store sandbox paths. Also handle both keychain
account names ("Slack" vs "Slack App Store Key") with smart ordering
based on which variant was detected.

Addresses PR #1 feedback.

Co-authored-by: Brandon P. <313913+brandon-dacrib@users.noreply.github.com>

https://claude.ai/code/session_01TPCTdT8Q7mKBW499t4DPJY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants