krz/octosentry
macOS menu bar app to monitor GitHub security alerts
clone: git clone https://gitbay.org/krz/octosentry.git
7b95fa2c1ebd2adffbf9df49067e032276ae0f64
signed_unknown_key
author: Christian Cleberg <hello@cleberg.net> · 2026-08-22T21:27:00Z
committer: <noreply@github.com>
octosentry/Account.swift | 55 ++++++++++ octosentry/AuthStore.swift | 156 ++++++++++++++++++++++++---- octosentry/GitHubAPIModels.swift | 5 + octosentry/GitHubSecurityAPIClient.swift | 9 ++ octosentry/KeychainTokenStore.swift | 11 +- octosentry/PersistedState.swift | 24 +++-- octosentry/SecurityEvent.swift | 3 + octosentry/SecurityEventListView.swift | 143 +++++++++++++++---------- octosentry/SecurityEventRow.swift | 12 +++ octosentry/SecurityEventStore.swift | 104 +++++++++++++++---- octosentryTests/AccountTests.swift | 131 +++++++++++++++++++++++ octosentryTests/PersistedStateTests.swift | 13 ++- octosentryTests/PersistenceStoreTests.swift | 2 +- 13 files changed, 561 insertions(+), 107 deletions(-) new file mode 100644 @@ -0,0 +1,55 @@ +// +// Account.swift +// octosentry +// +// A signed-in GitHub identity. The token itself stays in the Keychain; +// `keychainAccount` is the item name it lives under. +// +// Existing single-account installs keep the item they already have — +// `KeychainTokenStore.legacyAccount` is a perfectly good name for one +// account, and rewriting it at upgrade time would risk stranding a token to +// gain nothing. +// + +import Foundation + +nonisolated struct Account: Codable, Equatable, Identifiable, Hashable { + /// Stable across renames, unlike the login. Zero for an account carried + /// over from a single-account install before its login was resolved. + var id: Int + var login: String + var keychainAccount: String + var hasRepoScope: Bool + + var displayName: String { + login.isEmpty ? "GitHub account" : login + } + + /// The account an upgrading install already has a token for. + static func legacy() -> Account { + Account( + id: 0, + login: "", + keychainAccount: KeychainTokenStore.legacyAccount, + hasRepoScope: false + ) + } + + static func new(id: Int, login: String, hasRepoScope: Bool) -> Account { + Account( + id: id, + login: login, + keychainAccount: "account-\(id)", + hasRepoScope: hasRepoScope + ) + } +} + +/// A repo on the watch list, and the account whose token can see it. The same +/// repo can appear under more than one account; the feed merges those and +/// attributes them. +nonisolated struct WatchedRepo: Codable, Equatable, Hashable { + var fullName: String + /// Account id, or 0 for entries carried over from a single-account install. + var accountID: Int +} @@ -2,9 +2,9 @@ // AuthStore.swift // octosentry // -// Drives the device authorization flow and mirrors whether a token is -// currently in the Keychain. Replaces the GITHUB_TOKEN env var dev -// shortcut (spec §13) with the real v1 auth flow (spec §6). +// Drives the device authorization flow and tracks which GitHub identities +// are signed in. Each account's token lives in its own Keychain item; this +// holds only the account list, which is persisted alongside everything else. // // Sign-in requests the minimal security_events scope by default. // Broader "repo" scope (needed to list repos for the picker, #15) is @@ -19,17 +19,18 @@ import Observation final class AuthStore { private(set) var state: AuthState private(set) var errorMessage: String? - private(set) var hasRepoAccess = false + private(set) var accounts: [Account] = [] private let client = GitHubDeviceAuthClient() private let persistenceStore = PersistenceStore() private var authorizationTask: Task<Void, Never>? init() { + // Before the account list loads, fall back to whether the + // single-account Keychain item exists, so an upgrading install isn't + // shown a sign-in screen it doesn't need. state = KeychainTokenStore.load() != nil ? .signedIn : .signedOut - Task { - hasRepoAccess = await persistenceStore.load().hasRepoScope - } + Task { await loadAccounts() } } var isSignedIn: Bool { @@ -37,10 +38,21 @@ final class AuthStore { return false } + /// True when any signed-in account can list repos for the picker. + var hasRepoAccess: Bool { + accounts.contains(where: \.hasRepoScope) + } + func signIn() { beginAuthorization(scope: GitHubDeviceAuthClient.defaultScope) } + /// Adds another identity. Same flow as signing in — GitHub decides which + /// account authorizes the code. + func addAccount() { + beginAuthorization(scope: GitHubDeviceAuthClient.defaultScope) + } + /// Re-runs device auth with broader scope so the repo picker can list /// repos. Only called explicitly from the repo picker UI, never on /// the default sign-in path. @@ -48,12 +60,87 @@ final class AuthStore { beginAuthorization(scope: GitHubDeviceAuthClient.repoAccessScope) } - func signOut() { + /// Signs out one account, leaving the others alone. + func signOut(_ account: Account) async { + KeychainTokenStore.delete(account: account.keychainAccount) + + var persisted = await persistenceStore.load() + persisted.accounts.removeAll { $0.keychainAccount == account.keychainAccount } + // Its repos can no longer be fetched, so drop them too. + persisted.watchedRepos.removeAll { $0.accountID == account.id } + await persistenceStore.save(persisted) + + accounts = persisted.accounts + state = accounts.isEmpty ? .signedOut : .signedIn + } + + func signOutAll() async { + for account in accounts { + KeychainTokenStore.delete(account: account.keychainAccount) + } + // Also clear the pre-multi-account item, in case no account row + // referenced it. + KeychainTokenStore.delete() + + var persisted = await persistenceStore.load() + persisted.accounts = [] + persisted.watchedRepos = [] + await persistenceStore.save(persisted) + authorizationTask?.cancel() authorizationTask = nil - KeychainTokenStore.delete() + accounts = [] state = .signedOut - hasRepoAccess = false + } + + /// Reconciles the stored account list with the Keychain, and adopts a + /// token left by a single-account install. + private func loadAccounts() async { + var persisted = await persistenceStore.load() + + if persisted.accounts.isEmpty, KeychainTokenStore.load() != nil { + var legacy = Account.legacy() + legacy.hasRepoScope = persisted.hasRepoScope + persisted.accounts = [legacy] + await persistenceStore.save(persisted) + } + + accounts = persisted.accounts + state = accounts.isEmpty ? .signedOut : .signedIn + + await resolveMissingLogins() + } + + /// An adopted legacy account has no login until we ask GitHub who it is. + private func resolveMissingLogins() async { + let unresolved = accounts.filter { $0.login.isEmpty } + guard !unresolved.isEmpty else { return } + + var persisted = await persistenceStore.load() + var changed = false + + for account in unresolved { + guard let token = KeychainTokenStore.load(account: account.keychainAccount), + let user = try? await GitHubSecurityAPIClient(token: token).fetchCurrentUser(), + let index = persisted.accounts.firstIndex(where: { $0.keychainAccount == account.keychainAccount }) + else { continue } + + persisted.accounts[index].login = user.login + // Keep the keychain item where it is; only the identity is filled in. + if persisted.accounts[index].id == 0 { + let oldID = persisted.accounts[index].id + persisted.accounts[index].id = user.id + for repoIndex in persisted.watchedRepos.indices where persisted.watchedRepos[repoIndex].accountID == oldID { + persisted.watchedRepos[repoIndex].accountID = user.id + } + } + changed = true + } + + if changed { + await persistenceStore.save(persisted) + accounts = persisted.accounts + } } private func beginAuthorization(scope: String) { @@ -71,22 +158,51 @@ final class AuthStore { interval: deviceCode.interval, expiresIn: deviceCode.expiresIn ) - try KeychainTokenStore.save(token) - - let grantedRepoScope = scope.contains("repo") - var persisted = await persistenceStore.load() - persisted.hasRepoScope = grantedRepoScope - await persistenceStore.save(persisted) - hasRepoAccess = grantedRepoScope - + try await register(token: token, grantedRepoScope: scope.contains("repo")) state = .signedIn } catch { errorMessage = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription // A failed re-auth (e.g. requestRepoAccess while already // signed in) shouldn't sign the user out of their existing - // valid token — only reflect reality from the Keychain. - state = KeychainTokenStore.load() != nil ? .signedIn : .signedOut + // valid token — only reflect reality. + state = accounts.isEmpty && KeychainTokenStore.load() == nil ? .signedOut : .signedIn } } } + + /// Stores a freshly authorized token under its own Keychain item and + /// records the account. Re-authorizing an account already present updates + /// it in place rather than adding a duplicate. + private func register(token: String, grantedRepoScope: Bool) async throws { + let user = try await GitHubSecurityAPIClient(token: token).fetchCurrentUser() + + var persisted = await persistenceStore.load() + + if let index = persisted.accounts.firstIndex(where: { $0.id == user.id }) { + persisted.accounts[index].login = user.login + persisted.accounts[index].hasRepoScope = grantedRepoScope + try KeychainTokenStore.save(token, account: persisted.accounts[index].keychainAccount) + } else if let index = persisted.accounts.firstIndex(where: { $0.id == 0 }) { + // The adopted single-account entry, now identified. + let keychainAccount = persisted.accounts[index].keychainAccount + persisted.accounts[index] = Account( + id: user.id, + login: user.login, + keychainAccount: keychainAccount, + hasRepoScope: grantedRepoScope + ) + for repoIndex in persisted.watchedRepos.indices where persisted.watchedRepos[repoIndex].accountID == 0 { + persisted.watchedRepos[repoIndex].accountID = user.id + } + try KeychainTokenStore.save(token, account: keychainAccount) + } else { + let account = Account.new(id: user.id, login: user.login, hasRepoScope: grantedRepoScope) + try KeychainTokenStore.save(token, account: account.keychainAccount) + persisted.accounts.append(account) + } + + persisted.hasRepoScope = persisted.accounts.contains(where: \.hasRepoScope) + await persistenceStore.save(persisted) + accounts = persisted.accounts + } } @@ -87,6 +87,11 @@ nonisolated struct SecretScanningAlertDTO: Decodable { } } +nonisolated struct GitHubUserDTO: Decodable { + let id: Int + let login: String +} + nonisolated struct GitHubRepoDTO: Decodable { let fullName: String @@ -103,6 +103,15 @@ actor GitHubSecurityAPIClient { return dtos.map(\.fullName) } + /// Identifies whose token this is, so accounts can be told apart and + /// alerts attributed. Needs no scope beyond a valid user token. + func fetchCurrentUser() async throws -> GitHubUserDTO { + var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false)! + components.path = "/user" + let (data, _) = try await fetchData(url: components.url!) + return try decode(data) + } + private func alertsURL(owner: String, repo: String, path: String) -> URL { var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false)! components.path = "/repos/\(owner)/\(repo)/\(path)" @@ -14,9 +14,12 @@ import Security nonisolated enum KeychainTokenStore { private static let service = "net.cleberg.octosentry.github-token" - private static let account = "github-oauth-token" - static func save(_ token: String) throws { + /// The item name used before octosentry supported more than one account. + /// Still the name for that account's token — upgrading does not move it. + static let legacyAccount = "github-oauth-token" + + static func save(_ token: String, account: String = legacyAccount) throws { let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, @@ -35,7 +38,7 @@ nonisolated enum KeychainTokenStore { } } - static func load() -> String? { + static func load(account: String = legacyAccount) -> String? { let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, @@ -50,7 +53,7 @@ nonisolated enum KeychainTokenStore { return String(data: data, encoding: .utf8) } - static func delete() { + static func delete(account: String = legacyAccount) { let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, @@ -2,7 +2,8 @@ // PersistedState.swift // octosentry // -// Everything the app remembers across launches: the repo watch list, +// Everything the app remembers across launches: the signed-in accounts and +// the repo watch list, // local-only seen-state per event, last-fetch timestamp per repo, the // minimum severity filter, the feed sort order, local triage state, the // per-repo alert IDs the notifier has already accounted for, and whether the current token @@ -15,7 +16,8 @@ import Foundation nonisolated struct PersistedState: Codable { - var watchedRepos: [String] + var accounts: [Account] + var watchedRepos: [WatchedRepo] var seenEventIDs: Set<String> var lastFetchByRepo: [String: Date] var minimumSeverity: SecurityEventSeverity @@ -38,11 +40,12 @@ nonisolated struct PersistedState: Codable { enum CodingKeys: String, CodingKey { case watchedRepos, seenEventIDs, lastFetchByRepo, minimumSeverity, hasRepoScope, sortOrder - case notifiedEventIDsByRepo, triage, history + case notifiedEventIDsByRepo, triage, history, accounts } init( - watchedRepos: [String], + accounts: [Account] = [], + watchedRepos: [WatchedRepo], seenEventIDs: Set<String>, lastFetchByRepo: [String: Date], minimumSeverity: SecurityEventSeverity, @@ -52,6 +55,7 @@ nonisolated struct PersistedState: Codable { triage: AlertTriage = AlertTriage(), history: AlertHistory = AlertHistory() ) { + self.accounts = accounts self.watchedRepos = watchedRepos self.seenEventIDs = seenEventIDs self.lastFetchByRepo = lastFetchByRepo @@ -67,7 +71,15 @@ nonisolated struct PersistedState: Codable { // and sortOrder existed still load instead of falling back to .placeholder. init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) - watchedRepos = try container.decode([String].self, forKey: .watchedRepos) + accounts = try container.decodeIfPresent([Account].self, forKey: .accounts) ?? [] + // Before multi-account, watchedRepos was a plain [String] belonging to + // the one signed-in account. Decode either shape. + if let repos = try? container.decode([WatchedRepo].self, forKey: .watchedRepos) { + watchedRepos = repos + } else { + let names = try container.decode([String].self, forKey: .watchedRepos) + watchedRepos = names.map { WatchedRepo(fullName: $0, accountID: 0) } + } seenEventIDs = try container.decode(Set<String>.self, forKey: .seenEventIDs) lastFetchByRepo = try container.decode([String: Date].self, forKey: .lastFetchByRepo) minimumSeverity = try container.decode(SecurityEventSeverity.self, forKey: .minimumSeverity) @@ -82,7 +94,7 @@ nonisolated struct PersistedState: Codable { } static let placeholder = PersistedState( - watchedRepos: ["ccleberg/cleberg.net"], + watchedRepos: [WatchedRepo(fullName: "ccleberg/cleberg.net", accountID: 0)], seenEventIDs: [], lastFetchByRepo: [:], minimumSeverity: .low @@ -16,4 +16,7 @@ nonisolated struct SecurityEvent: Identifiable, Codable, Sendable { let createdAt: Date let updatedAt: Date var seenLocally: Bool + /// Logins of the accounts whose token can see this alert. More than one + /// when the same repo is watched under several identities. + var accountLogins: [String] = [] } @@ -300,6 +300,9 @@ struct SecurityEventListView: View { ForEach(store.events) { event in SecurityEventRow( event: event, + attribution: store.showsAttribution(for: event.repoFullName) + ? event.accountLogins.sorted().joined(separator: ", ") + : nil, isHidden: store.triage.isHidden(event.id, now: .now), snoozedUntil: store.triage.snoozedUntil(event.id, now: .now), onMarkSeen: { Task { await store.markSeen(event.id) } }, @@ -321,28 +324,76 @@ private struct RepoManagerView: View { var store: SecurityEventStore var authStore: AuthStore @State private var newRepoText = "" - @State private var isBrowsingRepos = false + @State private var addingToAccountID: Int? + @State private var browsingAccount: Account? @State private var availableRepos: [String] = [] @State private var isLoadingRepos = false @State private var browseErrorMessage: String? var body: some View { - VStack(alignment: .leading, spacing: 10) { - Text("Watched Repositories") - .font(.subheadline.weight(.semibold)) + ScrollView { + VStack(alignment: .leading, spacing: 12) { + ForEach(authStore.accounts) { account in + accountSection(account) + Divider() + } + + Button { + authStore.addAccount() + } label: { + Label("Add another account", systemImage: "person.badge.plus") + .font(.caption) + } + .buttonStyle(.plain) + .foregroundStyle(Color.accentColor) + + if let errorMessage = store.watchListErrorMessage { + Text(errorMessage) + .font(.caption2) + .foregroundStyle(.red) + } - if store.watchedRepos.isEmpty { - Text("No repos watched yet.") - .font(.callout) + Divider() + + Button("Sign Out of All Accounts") { + Task { await authStore.signOutAll() } + } + .buttonStyle(.plain) + .foregroundStyle(.red) + } + .padding(12) + .frame(maxWidth: .infinity, alignment: .leading) + } + } + + @ViewBuilder + private func accountSection(_ account: Account) -> some View { + VStack(alignment: .leading, spacing: 8) { + HStack { + Text(account.displayName) + .font(.subheadline.weight(.semibold)) + Spacer() + Button("Sign out") { + Task { await authStore.signOut(account) } + } + .buttonStyle(.plain) + .font(.caption) + .foregroundStyle(.red) + } + + let repos = store.watchedRepos.filter { $0.accountID == account.id } + if repos.isEmpty { + Text("No repos watched under this account.") + .font(.caption) .foregroundStyle(.secondary) } else { - ForEach(store.watchedRepos, id: \.self) { repo in + ForEach(repos, id: \.self) { watched in HStack { - Text(repo) + Text(watched.fullName) .font(.callout) Spacer() Button { - Task { await store.removeRepo(repo) } + Task { await store.removeRepo(watched) } } label: { Image(systemName: "minus.circle.fill") .foregroundStyle(.red) @@ -352,57 +403,41 @@ private struct RepoManagerView: View { } } - Divider() - - if isBrowsingRepos { - browsingContent + if browsingAccount == account { + browsingContent(account) } else { HStack { - TextField("owner/repo", text: $newRepoText) - .textFieldStyle(.roundedBorder) - .onSubmit(addRepo) - - Button("Add", action: addRepo) - .disabled(newRepoText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) + TextField("owner/repo", text: Binding( + get: { addingToAccountID == account.id ? newRepoText : "" }, + set: { newRepoText = $0; addingToAccountID = account.id } + )) + .textFieldStyle(.roundedBorder) + .onSubmit { addRepo(to: account) } + + Button("Add") { addRepo(to: account) } + .disabled(addingToAccountID != account.id + || newRepoText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) } - Button(action: startBrowsing) { - Label("Browse your repos", systemImage: "list.bullet") + Button { startBrowsing(account) } label: { + Label("Browse repos", systemImage: "list.bullet") .font(.caption) } .buttonStyle(.plain) .foregroundStyle(Color.accentColor) } - - if let errorMessage = store.watchListErrorMessage { - Text(errorMessage) - .font(.caption2) - .foregroundStyle(.red) - } - - Spacer() - - Divider() - - Button("Sign Out") { - authStore.signOut() - } - .buttonStyle(.plain) - .foregroundStyle(.red) } - .padding(12) - .frame(maxWidth: .infinity, alignment: .leading) } @ViewBuilder - private var browsingContent: some View { + private func browsingContent(_ account: Account) -> some View { VStack(alignment: .leading, spacing: 6) { HStack { - Text("Your Repositories") + Text("Repositories") .font(.caption.weight(.semibold)) Spacer() Button { - isBrowsingRepos = false + browsingAccount = nil } label: { Image(systemName: "xmark.circle") } @@ -418,7 +453,10 @@ private struct RepoManagerView: View { .font(.caption2) .foregroundStyle(.red) } else { - let selectableRepos = availableRepos.filter { !store.watchedRepos.contains($0) } + let watched = Set( + store.watchedRepos.filter { $0.accountID == account.id }.map(\.fullName) + ) + let selectableRepos = availableRepos.filter { !watched.contains($0) } if selectableRepos.isEmpty { Text("All visible repos are already watched.") .font(.caption2) @@ -428,8 +466,8 @@ private struct RepoManagerView: View { LazyVStack(alignment: .leading, spacing: 4) { ForEach(selectableRepos, id: \.self) { repo in Button { - Task { await store.addRepo(repo) } - isBrowsingRepos = false + Task { await store.addRepo(repo, accountID: account.id) } + browsingAccount = nil } label: { Text(repo) .font(.callout) @@ -445,17 +483,17 @@ private struct RepoManagerView: View { } } - private func startBrowsing() { - guard authStore.hasRepoAccess else { + private func startBrowsing(_ account: Account) { + guard account.hasRepoScope else { authStore.requestRepoAccess() return } - isBrowsingRepos = true + browsingAccount = account isLoadingRepos = true browseErrorMessage = nil Task { do { - availableRepos = try await store.fetchAccessibleRepos() + availableRepos = try await store.fetchAccessibleRepos(for: account) } catch { browseErrorMessage = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription } @@ -463,10 +501,11 @@ private struct RepoManagerView: View { } } - private func addRepo() { + private func addRepo(to account: Account) { let text = newRepoText newRepoText = "" - Task { await store.addRepo(text) } + addingToAccountID = nil + Task { await store.addRepo(text, accountID: account.id) } } } @@ -8,6 +8,9 @@ import SwiftUI struct SecurityEventRow: View { let event: SecurityEvent + /// Which account(s) this alert came through. Set only when the same repo + /// is watched under more than one identity — otherwise it's noise. + var attribution: String? var isHidden = false var snoozedUntil: Date? var onMarkSeen: () -> Void @@ -45,6 +48,15 @@ struct SecurityEventRow: View { .font(.caption) .foregroundStyle(.secondary) + if let attribution { + Text(attribution) + .font(.caption2) + .foregroundStyle(.secondary) + .padding(.horizontal, 5) + .padding(.vertical, 1) + .background(.secondary.opacity(0.15), in: Capsule()) + } + Spacer() if let hiddenLabel { @@ -26,7 +26,7 @@ final class SecurityEventStore { private(set) var minimumSeverity: SecurityEventSeverity = .low private(set) var sortOrder: AlertSortOrder = .severity private(set) var totalFetchedCount = 0 - private(set) var watchedRepos: [String] = [] + private(set) var watchedRepos: [WatchedRepo] = [] private(set) var watchListErrorMessage: String? /// Per-session narrowing, not persisted. Setting it re-derives `events`. @@ -48,6 +48,14 @@ final class SecurityEventStore { Set(rawEvents.map(\.repoFullName)).sorted { $0.localizedCaseInsensitiveCompare($1) == .orderedAscending } } + /// Repo names watched under more than one account — the feed shows + /// attribution only for these, since it's noise everywhere else. + private var reposWatchedBySeveralAccounts: Set<String> = [] + + func showsAttribution(for repoFullName: String) -> Bool { + reposWatchedBySeveralAccounts.contains(repoFullName) + } + /// Alerts held back by the source/repo filter, as opposed to the /// severity floor, so the empty state can say which one is hiding them. var filteredOutCount: Int { @@ -76,14 +84,13 @@ final class SecurityEventStore { triage = state.triage history = state.history - guard let token = KeychainTokenStore.load() else { + let accountsByID = Dictionary(uniqueKeysWithValues: state.accounts.map { ($0.id, $0) }) + guard !accountsByID.isEmpty else { errorMessages = [stateLoadFailure, GitHubAPIError.missingToken.errorDescription ?? "Not signed in."] .compactMap { $0 } return } - let client = GitHubSecurityAPIClient(token: token) - var fetchedEvents: [SecurityEvent] = [] var errors: [String] = [stateLoadFailure].compactMap { $0 } var notices: [String] = [] @@ -92,19 +99,30 @@ final class SecurityEventStore { // alerts look new on the next poll. var fetchedEventsByRepo: [String: [SecurityEvent]] = [:] - for repoFullName in state.watchedRepos { + for watched in state.watchedRepos { + let repoFullName = watched.fullName let parts = repoFullName.split(separator: "/", maxSplits: 1) guard parts.count == 2 else { continue } let owner = String(parts[0]) let repo = String(parts[1]) - async let dependabot = fetchSource(label: "\(repoFullName) · Dependabot") { + guard let account = accountsByID[watched.accountID], + let token = KeychainTokenStore.load(account: account.keychainAccount) else { + errors.append("\(repoFullName): no signed-in account can reach this repo.") + continue + } + let client = GitHubSecurityAPIClient(token: token) + let label = accountsByID.count > 1 + ? "\(repoFullName) (\(account.displayName))" + : repoFullName + + async let dependabot = fetchSource(label: "\(label) · Dependabot") { try await client.fetchDependabotAlerts(owner: owner, repo: repo) } - async let codeScanning = fetchSource(label: "\(repoFullName) · Code scanning") { + async let codeScanning = fetchSource(label: "\(label) · Code scanning") { try await client.fetchCodeScanningAlerts(owner: owner, repo: repo) } - async let secretScanning = fetchSource(label: "\(repoFullName) · Secret scanning") { + async let secretScanning = fetchSource(label: "\(label) · Secret scanning") { try await client.fetchSecretScanningAlerts(owner: owner, repo: repo) } @@ -114,7 +132,11 @@ final class SecurityEventStore { for outcome in outcomes { switch outcome { case .events(let sourceEvents): - repoEvents += sourceEvents + repoEvents += sourceEvents.map { event in + var event = event + event.accountLogins = [account.displayName] + return event + } repoSucceeded = true case .unavailable(let label): notices.append("\(label) alerts aren't available for this repo (disabled, or token lacks that permission).") @@ -125,10 +147,18 @@ final class SecurityEventStore { fetchedEvents += repoEvents if repoSucceeded { state.lastFetchByRepo[repoFullName] = Date() - fetchedEventsByRepo[repoFullName] = repoEvents + fetchedEventsByRepo[repoFullName, default: []] += repoEvents } } + // The same alert reached through two identities is one alert; merge + // the attributions rather than showing it twice. + fetchedEvents = Self.merged(fetchedEvents) + for (repoFullName, events) in fetchedEventsByRepo { + fetchedEventsByRepo[repoFullName] = Self.merged(events) + } + reposWatchedBySeveralAccounts = Self.reposWatchedBySeveralAccounts(in: state.watchedRepos) + rawEvents = fetchedEvents.map { event in var event = event event.seenLocally = state.seenEventIDs.contains(event.id) @@ -142,7 +172,7 @@ final class SecurityEventStore { // Only prune against a complete picture: if a repo failed this round // its alerts are missing, and pruning would forget they were hidden. - if fetchedEventsByRepo.count == state.watchedRepos.count { + if fetchedEventsByRepo.count == Set(state.watchedRepos.map(\.fullName)).count { let now = Date() state.triage = state.triage.pruned( presentEventIDs: Set(fetchedEvents.map(\.id)), @@ -164,7 +194,7 @@ final class SecurityEventStore { ) state.notifiedEventIDsByRepo = AlertDiff.updatedBaseline( from: fetchedEventsByRepo, - watchedRepos: state.watchedRepos, + watchedRepos: state.watchedRepos.map(\.fullName), previous: state.notifiedEventIDsByRepo ) await persistenceStore.save(state) @@ -190,7 +220,7 @@ final class SecurityEventStore { await persistenceStore.save(state) } - func addRepo(_ input: String) async { + func addRepo(_ input: String, accountID: Int) async { watchListErrorMessage = nil let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines) let parts = trimmed.split(separator: "/", omittingEmptySubsequences: true) @@ -205,14 +235,15 @@ final class SecurityEventStore { var state = await persistenceStore.load() // GitHub owner/repo names are case-insensitive, so treat entries - // that differ only in case as the same watched repo. + // that differ only in case as the same watched repo. The same repo + // under a different account is a separate entry on purpose. guard !state.watchedRepos.contains(where: { - $0.caseInsensitiveCompare(repoFullName) == .orderedSame + $0.accountID == accountID && $0.fullName.caseInsensitiveCompare(repoFullName) == .orderedSame }) else { watchListErrorMessage = "\(repoFullName) is already watched." return } - state.watchedRepos.append(repoFullName) + state.watchedRepos.append(WatchedRepo(fullName: repoFullName, accountID: accountID)) await persistenceStore.save(state) watchedRepos = state.watchedRepos @@ -222,8 +253,8 @@ final class SecurityEventStore { /// Lists repos the current token can see, for the repo picker (#15). /// Requires broader repo-access scope — throws if the token only has /// the default security_events scope. - func fetchAccessibleRepos() async throws -> [String] { - guard let token = KeychainTokenStore.load() else { + func fetchAccessibleRepos(for account: Account) async throws -> [String] { + guard let token = KeychainTokenStore.load(account: account.keychainAccount) else { throw GitHubAPIError.missingToken } return try await GitHubSecurityAPIClient(token: token).fetchAccessibleRepos() @@ -265,10 +296,12 @@ final class SecurityEventStore { applyFilters() } - func removeRepo(_ repoFullName: String) async { + func removeRepo(_ watched: WatchedRepo) async { var state = await persistenceStore.load() - state.watchedRepos.removeAll { $0 == repoFullName } - state.lastFetchByRepo.removeValue(forKey: repoFullName) + state.watchedRepos.removeAll { $0 == watched } + if !state.watchedRepos.contains(where: { $0.fullName == watched.fullName }) { + state.lastFetchByRepo.removeValue(forKey: watched.fullName) + } await persistenceStore.save(state) watchedRepos = state.watchedRepos @@ -298,6 +331,35 @@ final class SecurityEventStore { events = sortOrder.sorted(filter.apply(to: admitted)) } + /// Collapses alerts that arrived through more than one account, keeping + /// one row and unioning the attributions. Input order is preserved. + nonisolated static func merged(_ events: [SecurityEvent]) -> [SecurityEvent] { + var order: [String] = [] + var byID: [String: SecurityEvent] = [:] + + for event in events { + if var existing = byID[event.id] { + for login in event.accountLogins where !existing.accountLogins.contains(login) { + existing.accountLogins.append(login) + } + byID[event.id] = existing + } else { + order.append(event.id) + byID[event.id] = event + } + } + + return order.compactMap { byID[$0] } + } + + nonisolated static func reposWatchedBySeveralAccounts(in watched: [WatchedRepo]) -> Set<String> { + var accountsByRepo: [String: Set<Int>] = [:] + for repo in watched { + accountsByRepo[repo.fullName, default: []].insert(repo.accountID) + } + return Set(accountsByRepo.filter { $0.value.count > 1 }.keys) + } + private enum SourceOutcome { case events([SecurityEvent]) case unavailable(label: String) new file mode 100644 @@ -0,0 +1,131 @@ +// +// AccountTests.swift +// octosentryTests +// + +import Foundation +import Testing +@testable import octosentry + +struct AccountTests { + + // An install that already has a token must keep using the Keychain item + // it's in — rewriting it at upgrade time risks stranding the token. + @Test func theLegacyAccountPointsAtTheExistingKeychainItem() { + let account = Account.legacy() + + #expect(account.keychainAccount == KeychainTokenStore.legacyAccount) + #expect(account.id == 0) + #expect(account.login.isEmpty) + } + + @Test func newAccountsGetTheirOwnKeychainItem() { + let first = Account.new(id: 1, login: "octocat", hasRepoScope: false) + let second = Account.new(id: 2, login: "hubot", hasRepoScope: true) + + #expect(first.keychainAccount != second.keychainAccount) + #expect(first.keychainAccount != KeychainTokenStore.legacyAccount) + #expect(second.hasRepoScope) + } + + @Test func displayNameFallsBackBeforeTheLoginIsKnown() { + #expect(Account.legacy().displayName == "GitHub account") + #expect(Account.new(id: 1, login: "octocat", hasRepoScope: false).displayName == "octocat") + } + + @Test func roundTripsThroughCodable() throws { + let account = Account.new(id: 7, login: "octocat", hasRepoScope: true) + let decoded = try JSONDecoder().decode(Account.self, from: try JSONEncoder().encode(account)) + + #expect(decoded == account) + } + + @Test func watchedRepoRoundTripsThroughCodable() throws { + let repo = WatchedRepo(fullName: "octocat/hello-world", accountID: 7) + let decoded = try JSONDecoder().decode(WatchedRepo.self, from: try JSONEncoder().encode(repo)) + + #expect(decoded == repo) + } +} + +struct MultiAccountFeedTests { + + private func event(_ id: String, repo: String = "octocat/hello-world", logins: [String]) -> SecurityEvent { + var event = TestEvents.event(id: id, repo: repo) + event.accountLogins = logins + return event + } + + // MARK: - Merging + + // The same alert reached through two identities is one alert. + @Test func duplicateAlertsCollapseAndUnionTheirAttributions() { + let merged = SecurityEventStore.merged([ + event("a", logins: ["octocat"]), + event("a", logins: ["hubot"]), + ]) + + #expect(merged.count == 1) + #expect(merged[0].accountLogins.sorted() == ["hubot", "octocat"]) + } + + @Test func mergingLeavesDistinctAlertsAlone() { + let merged = SecurityEventStore.merged([ + event("a", logins: ["octocat"]), + event("b", logins: ["octocat"]), + ]) + + #expect(merged.map(\.id) == ["a", "b"]) + } + + @Test func mergingPreservesInputOrder() { + let merged = SecurityEventStore.merged([ + event("b", logins: ["octocat"]), + event("a", logins: ["hubot"]), + event("b", logins: ["hubot"]), + ]) + + #expect(merged.map(\.id) == ["b", "a"]) + } + + @Test func mergingDoesNotRepeatTheSameLogin() { + let merged = SecurityEventStore.merged([ + event("a", logins: ["octocat"]), + event("a", logins: ["octocat"]), + ]) + + #expect(merged[0].accountLogins == ["octocat"]) + } + + @Test func mergingAnEmptyFeedIsEmpty() { + #expect(SecurityEventStore.merged([]).isEmpty) + } + + // MARK: - Attribution + + // Attribution is noise unless a repo is genuinely reachable two ways. + @Test func onlyReposWatchedUnderSeveralAccountsAreAttributed() { + let watched = [ + WatchedRepo(fullName: "octocat/shared", accountID: 1), + WatchedRepo(fullName: "octocat/shared", accountID: 2), + WatchedRepo(fullName: "octocat/solo", accountID: 1), + ] + + let attributed = SecurityEventStore.reposWatchedBySeveralAccounts(in: watched) + + #expect(attributed == ["octocat/shared"]) + } + + @Test func aRepoListedTwiceUnderOneAccountIsNotAttributed() { + let watched = [ + WatchedRepo(fullName: "octocat/solo", accountID: 1), + WatchedRepo(fullName: "octocat/solo", accountID: 1), + ] + + #expect(SecurityEventStore.reposWatchedBySeveralAccounts(in: watched).isEmpty) + } + + @Test func anEmptyWatchListAttributesNothing() { + #expect(SecurityEventStore.reposWatchedBySeveralAccounts(in: []).isEmpty) + } +} @@ -27,7 +27,11 @@ struct PersistedStateTests { @Test func roundTripsEveryField() throws { let fetchedAt = Date(timeIntervalSince1970: 1_785_000_000) let original = PersistedState( - watchedRepos: ["octocat/hello-world", "octocat/spoon-knife"], + accounts: [Account.new(id: 42, login: "octocat", hasRepoScope: true)], + watchedRepos: [ + WatchedRepo(fullName: "octocat/hello-world", accountID: 42), + WatchedRepo(fullName: "octocat/spoon-knife", accountID: 42), + ], seenEventIDs: ["dependabot-octocat/hello-world-1", "codeScanning-octocat/spoon-knife-7"], lastFetchByRepo: ["octocat/hello-world": fetchedAt], minimumSeverity: .high, @@ -48,6 +52,7 @@ struct PersistedStateTests { ) #expect(decoded.watchedRepos == original.watchedRepos) + #expect(decoded.accounts == original.accounts) #expect(decoded.seenEventIDs == original.seenEventIDs) #expect(decoded.lastFetchByRepo == original.lastFetchByRepo) #expect(decoded.minimumSeverity == original.minimumSeverity) @@ -65,7 +70,7 @@ struct PersistedStateTests { #expect(Set(object.keys) == [ "watchedRepos", "seenEventIDs", "lastFetchByRepo", "minimumSeverity", "hasRepoScope", "sortOrder", - "triage", "history", + "triage", "history", "accounts", ]) // notifiedEventIDsByRepo is optional and nil on the placeholder, so it // encodes to nothing rather than a null. @@ -85,7 +90,9 @@ struct PersistedStateTests { let state = try Self.decoder.decode(PersistedState.self, from: Data(legacy.utf8)) - #expect(state.watchedRepos == ["octocat/hello-world"]) + // The pre-multi-account shape was a plain [String]. + #expect(state.watchedRepos == [WatchedRepo(fullName: "octocat/hello-world", accountID: 0)]) + #expect(state.accounts.isEmpty) #expect(state.seenEventIDs == ["dependabot-octocat/hello-world-1"]) #expect(state.minimumSeverity == .medium) #expect(state.hasRepoScope == false) @@ -34,7 +34,7 @@ struct PersistenceStoreTests { defer { try? FileManager.default.removeItem(at: directory) } let saved = PersistedState( - watchedRepos: ["octocat/hello-world"], + watchedRepos: [WatchedRepo(fullName: "octocat/hello-world", accountID: 42)], seenEventIDs: ["dependabot-octocat/hello-world-1"], lastFetchByRepo: ["octocat/hello-world": Date(timeIntervalSince1970: 1_785_000_000)], minimumSeverity: .high,