From a7a87cf5fb0e5da8b6c5dbcfd7b193bc2bc2d329 Mon Sep 17 00:00:00 2001 From: Bakir Gracic Date: Thu, 15 Oct 2026 05:11:35 +0200 Subject: [PATCH 1/4] Fix accessibility of the 1.0 views at large text sizes and under VoiceOver At the largest accessibility text size, History's metadata collapsed into one-character columns and a staged file row pushed Send wider than the screen. History now stacks its row and Send scrolls, with its padding inside the scroll view so the Send as link switch is not clipped. FileRow puts the size under the name, and Add Files and Add Folder stack when they do not fit. VoiceOver: field errors and footnotes are hints on their controls, disabled controls say why, and results and the menu-bar clipboard notice are announced. The menu-bar progress reads a percent and a direction, History reads Sent or Received and a status title, and decorative icons and spinners are hidden. An invalid code is explained in words, small text buttons on Receive get 44 pt targets on iOS, and the number pad gets a Done button. known-issues.md: the Inspector and VoiceOver audit stays an owner step, with the full view list, and the two findings not fixed (the 38 pt paste button, fixed-size glyphs) are recorded. --- app/CrocApp/Views/AdvancedSettingsView.swift | 17 ++++ app/CrocApp/Views/CodeField.swift | 17 +++- app/CrocApp/Views/FileRow.swift | 44 +++++++-- app/CrocApp/Views/HistoryView.swift | 89 ++++++++++++++----- app/CrocApp/Views/MenuBarQuickSendView.swift | 25 +++++- app/CrocApp/Views/PowerSettingsSections.swift | 15 +++- app/CrocApp/Views/ReceiveView.swift | 50 ++++++++--- app/CrocApp/Views/SendView.swift | 77 +++++++++++----- app/CrocApp/Views/SettingsView.swift | 4 +- app/CrocApp/Views/StatusBlock.swift | 7 ++ app/CrocApp/Views/TransferStatusView.swift | 7 +- docs/known-issues.md | 8 +- 12 files changed, 282 insertions(+), 78 deletions(-) diff --git a/app/CrocApp/Views/AdvancedSettingsView.swift b/app/CrocApp/Views/AdvancedSettingsView.swift index 802c42f7..aa24c27a 100644 --- a/app/CrocApp/Views/AdvancedSettingsView.swift +++ b/app/CrocApp/Views/AdvancedSettingsView.swift @@ -11,6 +11,11 @@ struct AdvancedSettingsView: View { @Environment(AppSettings.self) private var settings @State private var confirmReset = false + private enum NumberField: Hashable { + case basePort, transfers + } + @FocusState private var focusedNumberField: NumberField? + var body: some View { @Bindable var settings = settings @@ -126,6 +131,7 @@ struct AdvancedSettingsView: View { format: .number.grouping(.never) ) .numberEntry() + .focused($focusedNumberField, equals: .basePort) } } AdvancedRow( @@ -139,6 +145,7 @@ struct AdvancedSettingsView: View { format: .number.grouping(.never) ) .numberEntry() + .focused($focusedNumberField, equals: .transfers) } } AdvancedRow( @@ -162,6 +169,15 @@ struct AdvancedSettingsView: View { .navigationTitle("Advanced") #if os(iOS) .scrollDismissesKeyboard(.interactively) + // The number pad has no Return key to dismiss it. + .toolbar { + ToolbarItemGroup(placement: .keyboard) { + if focusedNumberField != nil { + Spacer() + Button("Done") { focusedNumberField = nil } + } + } + } #endif .confirmationDialog( "Reset all Advanced settings?", isPresented: $confirmReset, titleVisibility: .visible @@ -202,6 +218,7 @@ private struct AdvancedRow: View { var body: some View { VStack(alignment: .leading, spacing: Spacing.space3) { content + .accessibilityHint(error ?? footnote) if let error { StatusBanner(kind: .error, title: error) .frame(maxWidth: .infinity, alignment: .leading) diff --git a/app/CrocApp/Views/CodeField.swift b/app/CrocApp/Views/CodeField.swift index 4bfe3cbe..9a5ab003 100644 --- a/app/CrocApp/Views/CodeField.swift +++ b/app/CrocApp/Views/CodeField.swift @@ -15,6 +15,9 @@ struct CodeField: View { let placeholder: String @Binding var text: String var isInvalid: Bool = false + /// Says in words what is wrong, shown under the field and read by + /// VoiceOver with it; the red border alone is not enough. + var invalidMessage: String? var isDisabled: Bool = false /// Receives the pasted string on the main actor; the call site decides /// what counts as a valid paste (e.g. ReceiveView's deeplink parsing). @@ -31,12 +34,24 @@ struct CodeField: View { private static let codeTracking: CGFloat = 0.5 // --tracking-code, typography.md var body: some View { + VStack(alignment: .leading, spacing: Spacing.space2) { + field + if isInvalid, let invalidMessage { + Text(invalidMessage) + .font(.caption) + .foregroundStyle(Color.textSecondary) + } + } + } + + private var field: some View { HStack(spacing: Spacing.space2) { TextField(placeholder, text: $text) .textFieldStyle(.plain) .font(.body.monospaced()) // --type-code-body, swiftui-mapping.md → Typography .tracking(Self.codeTracking) .focused($isFocused) + .accessibilityHint(isInvalid ? (invalidMessage ?? "") : "") .autocorrectionDisabled() #if os(iOS) .textInputAutocapitalization(.never) @@ -54,7 +69,7 @@ struct CodeField: View { } .padding(.leading, Self.leadingPadding) .padding(.trailing, Self.trailingPadding) - .frame(height: ControlHeight.large) + .frame(minHeight: ControlHeight.large) .background( Color.surfaceBase, in: RoundedRectangle(cornerRadius: Radius.md, style: .continuous) ) diff --git a/app/CrocApp/Views/FileRow.swift b/app/CrocApp/Views/FileRow.swift index 8d41bbcb..3e7c223a 100644 --- a/app/CrocApp/Views/FileRow.swift +++ b/app/CrocApp/Views/FileRow.swift @@ -18,6 +18,8 @@ struct FileRow: View { /// line 114: "the last 8 characters ... are pinned"). private static let pinnedTailLength = 8 + @Environment(\.dynamicTypeSize) private var dynamicTypeSize + var body: some View { HStack(spacing: Spacing.space3) { Image(systemName: typeSymbolName) @@ -25,23 +27,37 @@ struct FileRow: View { .foregroundStyle(Color.accentFill) .accessibilityHidden(true) - filenameLabel - .font(.callout) // typography.md: --type-callout is used for "file names" - .foregroundStyle(Color.textPrimary) + if dynamicTypeSize.isAccessibilitySize { + // The pinned-tail label keeps 8 characters unshrinkable, which + // is wider than the row at these sizes: wrap the name and put + // the size under it. + VStack(alignment: .leading, spacing: Spacing.space2) { + Text(url.lastPathComponent) + .font(.callout) + .foregroundStyle(Color.textPrimary) + .lineLimit(3) + .truncationMode(.middle) + sizeLabel + } + .frame(maxWidth: .infinity, alignment: .leading) + } else { + filenameLabel + .font(.callout) // typography.md: --type-callout is used for "file names" + .foregroundStyle(Color.textPrimary) - Spacer(minLength: Spacing.space3) + Spacer(minLength: Spacing.space3) - if let sizeText { - Text(sizeText) - .font(.footnote) - .foregroundStyle(Color.textSecondary) - .monospacedDigit() + sizeLabel } Button(action: onRemove) { Image(systemName: "xmark.circle.fill") .font(.system(size: Self.glyphSize)) .foregroundStyle(Color.textSecondary) + #if os(iOS) + .frame(minWidth: ControlHeight.hitTargetMin, minHeight: ControlHeight.hitTargetMin) + .contentShape(Rectangle()) + #endif } .buttonStyle(.plain) .accessibilityLabel("Remove \(url.lastPathComponent)") @@ -62,6 +78,16 @@ struct FileRow: View { .clipShape(rowShape) } + @ViewBuilder private var sizeLabel: some View { + if let sizeText { + Text(sizeText) + .font(.footnote) + .foregroundStyle(Color.textSecondary) + .monospacedDigit() + .lineLimit(1) + } + } + private var rowShape: some Shape { UnevenRoundedRectangle( topLeadingRadius: isFirst ? Radius.md : 0, diff --git a/app/CrocApp/Views/HistoryView.swift b/app/CrocApp/Views/HistoryView.swift index 15f9104c..f2a1bed1 100644 --- a/app/CrocApp/Views/HistoryView.swift +++ b/app/CrocApp/Views/HistoryView.swift @@ -133,40 +133,70 @@ struct HistoryView: View { private struct HistoryRow: View { let record: TransferRecord + @Environment(\.dynamicTypeSize) private var dynamicTypeSize + + private var isLargeText: Bool { dynamicTypeSize.isAccessibilitySize } var body: some View { - HStack(spacing: 12) { - Image(systemName: record.isSend ? "arrow.up.circle" : "arrow.down.circle") - .font(.title3) - .foregroundStyle(record.status == .completed ? Color.accentColor : .secondary) - VStack(alignment: .leading, spacing: 2) { - Text(title) - .lineLimit(1) - .truncationMode(.middle) - HStack(spacing: 8) { - Text(record.date, format: .relative(presentation: .named)) - if record.totalBytes > 0 { - Text(record.totalBytes.formatted(.byteCount(style: .file))) - } - if !record.codeHint.isEmpty { - Text(record.codeHint).monospaced() + Group { + if isLargeText { + // The glyph and badge sit above the text so it keeps the + // full row width instead of a few characters per line. + VStack(alignment: .leading, spacing: 8) { + HStack(spacing: 12) { + directionGlyph + statusBadge } + textColumn } - .font(.caption) - .foregroundStyle(.secondary) - if let linkState { - Text(linkState) - .font(.caption) - .foregroundStyle(.secondary) + } else { + HStack(spacing: 12) { + directionGlyph + textColumn + statusBadge } } - Spacer() - statusBadge } .padding(.vertical, 2) .accessibilityElement(children: .combine) } + private var directionGlyph: some View { + Image(systemName: record.isSend ? "arrow.up.circle" : "arrow.down.circle") + .font(.title3) + .accessibilityLabel(record.isSend ? "Sent" : "Received") + .foregroundStyle(record.status == .completed ? Color.accentColor : .secondary) + } + + private var textColumn: some View { + VStack(alignment: .leading, spacing: 2) { + Text(title) + .lineLimit(isLargeText ? 3 : 1) + .truncationMode(.middle) + let layout = + isLargeText + ? AnyLayout(VStackLayout(alignment: .leading, spacing: 2)) + : AnyLayout(HStackLayout(spacing: 8)) + layout { + Text(record.date, format: .relative(presentation: .named)) + if record.totalBytes > 0 { + Text(record.totalBytes.formatted(.byteCount(style: .file))) + } + if !record.codeHint.isEmpty { + Text(record.codeHint).monospaced() + } + } + .font(.caption) + .foregroundStyle(.secondary) + if let linkState { + Text(linkState) + .font(.caption) + .foregroundStyle(.secondary) + } + } + .frame(maxWidth: .infinity, alignment: .leading) + } + /// Where a stored upload's link stands. Revoked beats expired. private var linkState: String? { guard record.storedID != nil else { return nil } @@ -200,6 +230,17 @@ private struct HistoryRow: View { Image(systemName: "hand.raised.fill").foregroundStyle(.orange) } } - .accessibilityLabel(record.statusRaw) + .accessibilityLabel(record.status.title) + } +} + +extension TransferRecord.Status { + fileprivate var title: String { + switch self { + case .completed: "Completed" + case .failed: "Failed" + case .cancelled: "Cancelled" + case .declined: "Declined" + } } } diff --git a/app/CrocApp/Views/MenuBarQuickSendView.swift b/app/CrocApp/Views/MenuBarQuickSendView.swift index 6feefad0..80181b37 100644 --- a/app/CrocApp/Views/MenuBarQuickSendView.swift +++ b/app/CrocApp/Views/MenuBarQuickSendView.swift @@ -1,4 +1,5 @@ #if os(macOS) +import Accessibility import AppKit import CrocKit import SwiftUI @@ -28,7 +29,7 @@ struct MenuBarQuickSendView: View { idle case .starting, .connecting: VStack(spacing: Spacing.space3) { - ProgressView() + ProgressView().accessibilityHidden(true) Text("Connecting…").foregroundStyle(.secondary) cancelButton } @@ -103,7 +104,13 @@ struct MenuBarQuickSendView: View { HStack { Button("Choose Files…") { quickSend.chooseFiles() } - Button("Send Clipboard Text") { quickSend.sendClipboardText() } + Button("Send Clipboard Text") { + // A repeat tap leaves the notice unchanged, so an + // onChange would stay silent; announce per tap instead. + if !quickSend.sendClipboardText(), let notice = quickSend.notice { + AccessibilityNotification.Announcement(notice).post() + } + } } if let notice = quickSend.notice { Text(notice).font(.caption).foregroundStyle(.secondary) @@ -138,13 +145,25 @@ struct MenuBarQuickSendView: View { return VStack(spacing: Spacing.space3) { Text(p.fileName).lineLimit(1).truncationMode(.middle) ProgressView(value: Double(done), total: Double(max(p.totalSize, 1))) - .accessibilityLabel("Transfer progress") + .accessibilityLabel(progressLabel) + .accessibilityValue( + (Double(done) / Double(max(p.totalSize, 1))).formatted( + .percent.precision(.fractionLength(0)))) cancelButton } } private var direction: TransferController.Direction { controller.direction } + private var progressLabel: String { + switch (direction, controller.isStoredTransfer) { + case (.send, false): "Sending progress" + case (.send, true): "Uploading progress" + case (.receive, false): "Receiving progress" + case (.receive, true): "Downloading progress" + } + } + private var cancelButton: some View { Button("Cancel", role: .destructive) { controller.cancel() } } diff --git a/app/CrocApp/Views/PowerSettingsSections.swift b/app/CrocApp/Views/PowerSettingsSections.swift index 42518acb..d89f739d 100644 --- a/app/CrocApp/Views/PowerSettingsSections.swift +++ b/app/CrocApp/Views/PowerSettingsSections.swift @@ -7,6 +7,9 @@ import SwiftUI struct PowerSettingsSections: View { @Environment(AppSettings.self) private var settings + private static let connectionLockedReason = + "Unavailable while Local network only is on: those transfers never use a relay." + var body: some View { @Bindable var settings = settings @@ -41,6 +44,7 @@ struct PowerSettingsSections: View { .textInputAutocapitalization(.never) .keyboardType(.URL) #endif + .accessibilityHint(AppSettings.storedServerError(settings.storedServer) ?? "") if let error = AppSettings.storedServerError(settings.storedServer) { StatusBanner(kind: .error, title: error) } @@ -63,12 +67,17 @@ struct PowerSettingsSections: View { Text("Relay only").tag(Transport.relay) } .disabled(settings.onlyLocal) + .accessibilityHint(settings.onlyLocal ? Self.connectionLockedReason : "") } header: { Text("Connection") } footer: { - Text( - "Automatic can carry files of 128 MB or more directly between devices. Only the sender's choice applies; a receive follows it." - ) + if settings.onlyLocal { + Text(Self.connectionLockedReason) + } else { + Text( + "Automatic can carry files of 128 MB or more directly between devices. Only the sender's choice applies; a receive follows it." + ) + } } Section { diff --git a/app/CrocApp/Views/ReceiveView.swift b/app/CrocApp/Views/ReceiveView.swift index 8ea8ab20..c1db6971 100644 --- a/app/CrocApp/Views/ReceiveView.swift +++ b/app/CrocApp/Views/ReceiveView.swift @@ -47,7 +47,9 @@ struct ReceiveView: View { CodeField( placeholder: "Code phrase", text: $code, - isInvalid: !code.isEmpty && code.count < EngineConstraints.minCodeLength + isInvalid: !code.isEmpty && code.count < EngineConstraints.minCodeLength, + invalidMessage: + "A code is at least \(EngineConstraints.minCodeLength) characters." ) { pasted in if let link = TransferLink.parse(pasted) { router.open(link) } } @@ -84,17 +86,25 @@ struct ReceiveView: View { Image(systemName: "folder") } Spacer() - Button("Change") { showFolderPicker = true } - .fileImporter( - isPresented: $showFolderPicker, - allowedContentTypes: [.folder] - ) { result in - if case .success(let url) = result { - outputFolder.select(url) - } + Button { + showFolderPicker = true + } label: { + Text("Change").hitTarget() + } + .fileImporter( + isPresented: $showFolderPicker, + allowedContentTypes: [.folder] + ) { result in + if case .success(let url) = result { + outputFolder.select(url) } + } if outputFolder.isUserSelected { - Button("Reset") { outputFolder.resetToDefault() } + Button { + outputFolder.resetToDefault() + } label: { + Text("Reset").hitTarget() + } } } .font(.callout) @@ -132,7 +142,12 @@ struct ReceiveView: View { Label("Download link from \(host)", systemImage: "link") .lineLimit(2) Spacer() - Button("Clear") { storedLink = nil } + Button { + storedLink = nil + } label: { + Text("Clear").hitTarget() + } + .accessibilityLabel("Clear download link") } .padding(.horizontal, Spacing.space4) .frame(minHeight: ControlHeight.large) @@ -145,3 +160,16 @@ struct ReceiveView: View { ) } } + +extension View { + /// design/accessibility.md → Hit targets: bare text buttons still get a + /// 44pt target on iOS. macOS keeps its own control sizing. + fileprivate func hitTarget() -> some View { + #if os(iOS) + frame(minWidth: ControlHeight.hitTargetMin, minHeight: ControlHeight.hitTargetMin) + .contentShape(Rectangle()) + #else + self + #endif + } +} diff --git a/app/CrocApp/Views/SendView.swift b/app/CrocApp/Views/SendView.swift index c882c31a..486e3f85 100644 --- a/app/CrocApp/Views/SendView.swift +++ b/app/CrocApp/Views/SendView.swift @@ -6,6 +6,7 @@ struct SendView: View { @Environment(TransferController.self) private var controller @Environment(AppRouter.self) private var router @Environment(AppSettings.self) private var settings + @Environment(\.dynamicTypeSize) private var dynamicTypeSize enum Mode: String, CaseIterable { case files = "Files" @@ -53,7 +54,24 @@ struct SendView: View { .onChange(of: router.pendingSendURLs) { _, _ in consumePendingURLs() } } + // At accessibility text sizes the form is taller than a phone screen, so it + // scrolls there. Not ViewThatFits (as TransferStatusView uses): the keyboard + // shrinks the space, which would swap the branch and drop the text field's + // focus mid-typing. private var form: some View { + Group { + // Padding inside the scroll view: outside it, the scroll view's + // clip cuts off the Toggle switch, which draws past its frame. + if dynamicTypeSize.isAccessibilitySize { + ScrollView { formContent.padding() } + } else { + formContent.padding() + } + } + .frame(maxWidth: LayoutCap.contentMaxWidth) + } + + private var formContent: some View { VStack(spacing: Spacing.space5) { Picker("What to send", selection: $mode) { ForEach(Mode.allCases, id: \.self) { Text($0.rawValue) } @@ -105,8 +123,6 @@ struct SendView: View { .controlSize(.large) .disabled(!canStart) } - .padding() - .frame(maxWidth: LayoutCap.contentMaxWidth) } /// Why "Send as link" cannot be used for what is staged, or nil (R18). @@ -118,6 +134,15 @@ struct SendView: View { return nil } + /// What the banner and caption below the toggle say, read with the toggle. + private var linkToggleHint: String { + if let reason = linkUnavailableReason { return reason } + if linkSelected, let error = AppSettings.storedServerError(settings.storedServer) { + return error + " Change it in Settings." + } + return "" + } + private var linkSelected: Bool { sendAsLink && linkUnavailableReason == nil } private static func isFolder(_ url: URL) -> Bool { @@ -129,6 +154,7 @@ struct SendView: View { VStack(alignment: .leading, spacing: Spacing.space3) { Toggle("Send as link", isOn: Binding(get: { linkSelected }, set: { sendAsLink = $0 })) .disabled(linkUnavailableReason != nil) + .accessibilityHint(linkToggleHint) if let reason = linkUnavailableReason { Text(reason).font(.caption).foregroundStyle(.secondary) } else if linkSelected { @@ -230,35 +256,42 @@ struct SendView: View { isDropTargeted = $0 } - HStack { - Button("Add Files") { showFileImporter = true } - .fileImporter( - isPresented: $showFileImporter, - allowedContentTypes: [.item], - allowsMultipleSelection: true - ) { result in - if case .success(let urls) = result { - pickedURLs.append(contentsOf: urls.filter { !pickedURLs.contains($0) }) - } - } - Button("Add Folder") { showFolderImporter = true } - .fileImporter( - isPresented: $showFolderImporter, - allowedContentTypes: [.folder] - ) { result in - if case .success(let url) = result, !pickedURLs.contains(url) { - pickedURLs.append(url) - } - } + // Side by side until the labels no longer fit (accessibility sizes). + ViewThatFits(in: .horizontal) { + HStack { importButtons } + VStack { importButtons } } } } + @ViewBuilder private var importButtons: some View { + Button("Add Files") { showFileImporter = true } + .fileImporter( + isPresented: $showFileImporter, + allowedContentTypes: [.item], + allowsMultipleSelection: true + ) { result in + if case .success(let urls) = result { + pickedURLs.append(contentsOf: urls.filter { !pickedURLs.contains($0) }) + } + } + Button("Add Folder") { showFolderImporter = true } + .fileImporter( + isPresented: $showFolderImporter, + allowedContentTypes: [.folder] + ) { result in + if case .success(let url) = result, !pickedURLs.contains(url) { + pickedURLs.append(url) + } + } + } + // MARK: - Text private var textSection: some View { VStack(spacing: Spacing.space4) { TextEditor(text: $text) + .accessibilityLabel("Text to send") .font(.body.monospaced()) .frame(minHeight: 160, maxHeight: 280) .overlay { diff --git a/app/CrocApp/Views/SettingsView.swift b/app/CrocApp/Views/SettingsView.swift index a42910bc..2b60685f 100644 --- a/app/CrocApp/Views/SettingsView.swift +++ b/app/CrocApp/Views/SettingsView.swift @@ -89,7 +89,9 @@ extension SettingsView { Toggle( "Open at login", isOn: Binding( - get: { loginItem.isRegistered }, set: { loginItem.setRegistered($0) })) + get: { loginItem.isRegistered }, set: { loginItem.setRegistered($0) }) + ) + .accessibilityHint(loginItem.errorMessage ?? "") if loginItem.needsApproval { HStack { Text("Allow CrocApp in System Settings > General > Login Items.") diff --git a/app/CrocApp/Views/StatusBlock.swift b/app/CrocApp/Views/StatusBlock.swift index 205e79f0..0515c834 100644 --- a/app/CrocApp/Views/StatusBlock.swift +++ b/app/CrocApp/Views/StatusBlock.swift @@ -1,3 +1,4 @@ +import Accessibility import SwiftUI /// Large centered result (transfer done / failed). design/components.md → @@ -61,5 +62,11 @@ struct StatusBlock: View { } } .padding(Spacing.space6) + // Appears in place of the progress screen, so VoiceOver focus has + // nothing to land on; announce the outcome as StatusBanner does. + .onAppear { + let message = [title, detail].compactMap { $0 }.joined(separator: ". ") + AccessibilityNotification.Announcement(message).post() + } } } diff --git a/app/CrocApp/Views/TransferStatusView.swift b/app/CrocApp/Views/TransferStatusView.swift index ecbaa56a..3e4ec705 100644 --- a/app/CrocApp/Views/TransferStatusView.swift +++ b/app/CrocApp/Views/TransferStatusView.swift @@ -54,13 +54,13 @@ struct TransferStatusView: View { case .idle: EmptyView() case .starting: - ProgressView() + ProgressView().accessibilityHidden(true) Text("Starting…").foregroundStyle(.secondary) if showsSlowPhaseHint { slowPhaseHintText } case .connecting: - ProgressView() + ProgressView().accessibilityHidden(true) Text("Connecting…").foregroundStyle(.secondary) if showsSlowPhaseHint { slowPhaseHintText @@ -188,7 +188,7 @@ struct TransferStatusView: View { .multilineTextAlignment(.center) } TrustBadge(relay: controller.activeRelay) - ProgressView() + ProgressView().accessibilityHidden(true) Text("Waiting for the receiver…").font(.caption).foregroundStyle(.secondary) } } @@ -214,6 +214,7 @@ struct TransferStatusView: View { Image(systemName: "person.crop.circle.badge.checkmark") .font(.system(size: 40)) .foregroundStyle(.blue) + .accessibilityHidden(true) Text("Receiver connected").font(.headline) Text("Send the selected items to the connected device?") .foregroundStyle(.secondary) diff --git a/docs/known-issues.md b/docs/known-issues.md index eab4681b..94440099 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -26,11 +26,17 @@ Engine and bridge invariants live in `knowledge/crocmobile-bridge.md`; UI state- ## Release -- **The R29 Accessibility Inspector audit has not run for the P07-P09 views.** Owner step, before Submit for Review: audit the Keep Both / Replace sheet, the "When a file already exists" and Connection pickers, and the waiting screen's QR, Share Link and relay note with Accessibility Inspector (and VoiceOver). What was checked instead (2026-10-14, iPhone 17 Pro simulator at accessibility-extra-extra-extra-large, screenshots only, no VoiceOver): the sheet's buttons stack, the status screens scroll, the code wraps inside its glass shape, the Connection and file-policy pickers read in full. Still open there: a long file name in the incoming list is middle-truncated by design (one line). macOS layout of the changed views was not looked at after the change. +- **The R29 Accessibility Inspector and VoiceOver audit has not run on any 1.0 view.** Owner step O12, before Submit for Review: audit every view new or changed since 0.9.9 with Accessibility Inspector and VoiceOver, on iOS and macOS: the Keep Both / Replace sheet, the "When a file already exists" and Connection pickers, the waiting screen's QR, Share Link and relay note, Send as link and its result screen, the stored-link row on Receive, History (Revoke, the row layout), the stored-server field, the Advanced settings page, and the menu-bar window. No session can drive Inspector: computer use was not available in B23 (2026-10-15). What was checked instead: + - **2026-10-14:** iPhone 17 Pro simulator at accessibility-extra-extra-extra-large, screenshots only. The sheet's buttons stack, the status screens scroll, the code wraps inside its glass shape, and the Connection and file-policy pickers read in full. A long file name in the incoming list is middle-truncated by design (one line). + - **2026-10-15 (B23):** a code pass over every 1.0 view against `design/accessibility.md`, with its findings fixed, then the same simulator size for Send (empty and staged), Receive, Settings, Advanced, History and How it works. History and Send had overflowing layouts, now fixed. Send's staged-file list keeps its 280 pt height cap and scrolls inside the page at that size. + - **To check by ear:** whether VoiceOver reads the hint and the visible footnote or error of an Advanced field twice (the hint is there so the text is read with the field), whether a `decorative` QR image with a label is exposed as an image, and Full Keyboard Access focus in the menu-bar window when its content changes. + - **Not looked at:** macOS layout of the changed views. - **The App Store and direct-download builds cannot both be installed in /Applications.** Both are named `CrocApp.app` (`PRODUCT_NAME = CrocApp` on `CrocApp Direct`, so the menu bar, About and Quit say "CrocApp"). The separate bundle ID keeps their data apart, but dragging the DMG copy into /Applications replaces the App Store copy. What the App Store does when a direct-download `CrocApp.app` is already there is untested. One channel per Mac. ## Design system +- **The paste button in `CodeField` is 38 pt, below the 44 pt hit-target rule.** The component spec in `CodeField.swift` sets it, and `design/accessibility.md` says 44 pt for anything interactive. One of the two documents has to change. Found in the B23 accessibility pass, not changed there. +- **Small status glyphs use fixed point sizes.** `StatusBanner`, `StatusBlock`, `TrustBadge`, `FileRow`, `OnboardingView`, `ContentView` and the direction arrow in `TransferStatusView` set `.font(.system(size:))`, so they do not track Dynamic Type, against `design/accessibility.md` ("App glyphs size by text style"). They are decorative or repeat a word next to them, so nothing is lost at large sizes. Predates 1.0; found in the B23 accessibility pass. - **`FileRow` stats each file for its size label inside `body`.** `List` is lazy so only visible rows pay, but it is still synchronous I/O on the main actor during layout, and a network or iCloud volume could make it hitch. Not measured. ## Landing page From 9960bb5ef3cb41819da05c06b1561e13b7fd40f0 Mon Sep 17 00:00:00 2001 From: Bakir Gracic Date: Thu, 15 Oct 2026 06:21:36 +0200 Subject: [PATCH 2/4] Fix the security scan findings: glob, queued answers, path splitting, size overflow A picked file named with '*' was expanded by croc's filepath.Glob, so a send also leaked every matching file beside it. Picked paths are now escaped and match only themselves. After a manual accept, crocmobile queued "y" answers for the unzip and empty-folder prompts. croc v11 re-runs the accept prompt after a reconnect, so a sender could get an unseen file list accepted with Overwrite. Only "n" is queued now: Replace still overwrites files, but keeps existing members of a zipped folder, and a reconnect after a manual accept ends as refused. A path containing a line break split into extra send paths across the bridge, and the Go side trimmed each path, so "report.pdf " sent "report.pdf". CrocEngine refuses line breaks, the Go side splits without trimming, and received or shared names with control characters are unsafe. Sender-declared sizes could overflow the progress sums and trap the app. Every reported size and sum is capped at 1 PiB. --- CHANGELOG.md | 7 ++ CrocKit/Sources/CrocKit/CrocEngine.swift | 14 ++- .../SendPathValidationTests.swift | 37 ++++++++ app/CrocApp/Support/ReceivedName.swift | 5 +- app/CrocAppTests/ReceivedNameTests.swift | 1 + app/CrocShare/ShareStager.swift | 5 +- crocmobile/crocmobile.go | 19 ++-- crocmobile/session.go | 67 +++++++++----- crocmobile/session_test.go | 88 ++++++++++++++++++- crocmobile/store.go | 2 +- docs/knowledge/app-ui-architecture.md | 2 +- docs/knowledge/crocmobile-bridge.md | 2 +- docs/known-issues.md | 4 +- scripts/verify-interop.sh | 16 ++-- web/docs/content/guide/receiving.md | 4 +- 15 files changed, 220 insertions(+), 53 deletions(-) create mode 100644 CrocKit/Tests/CrocKitTests/SendPathValidationTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ed77a68..a8dfeb90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,13 @@ the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - A receive no longer overwrites existing files by default. A zipped folder received into a folder that already holds it no longer fails. Answers left over from one transfer can no longer answer the next transfer's accept prompt. +### Security + +- Sending a file whose name contains `*` sends only that file. Before, croc read the name as a pattern and also sent every matching file next to it. +- A file name containing a line break can no longer turn into extra send paths (a crafted name could send the app's whole data folder), and a name ending in a space is no longer sent as the file without the space (croc 11 receivers refuse such a name, so that send now fails). Received or shared names with control characters are treated as unsafe. +- After you accept a receive, a sender that reconnects cannot get a different file list accepted without you seeing it: the receive ends instead. Replace still overwrites files, but no longer overwrites existing files inside a zipped folder; they are kept. +- A sender declaring impossible file sizes can no longer crash the app during a receive. + ## [0.9.9] - 2026-07-30 First public build of CrocApp. It is a symbolic release: the app does everything V1 is meant to do, but it is not signed with an Apple Developer ID and it is not notarized, so macOS refuses to open it until you allow it by hand. Read "Opening it the first time" below before downloading. diff --git a/CrocKit/Sources/CrocKit/CrocEngine.swift b/CrocKit/Sources/CrocKit/CrocEngine.swift index 9eaa1f0d..b39849f7 100644 --- a/CrocKit/Sources/CrocKit/CrocEngine.swift +++ b/CrocKit/Sources/CrocKit/CrocEngine.swift @@ -31,7 +31,7 @@ public actor CrocEngine { paths: [String], text: String?, options: EngineOptions ) throws -> EngineTransfer { let opts = try Self.crocOptions(from: options) - let joined = paths.joined(separator: "\n") + let joined = try Self.joinedPaths(paths) return try start { bridge, error in CrocmobileStartSend(joined, text ?? "", opts, bridge, error) } @@ -55,7 +55,7 @@ public actor CrocEngine { opts.lifetimeSeconds = options.lifetimeSeconds opts.socks5Proxy = options.proxy.socks5 opts.httpProxy = options.proxy.http - let joined = paths.joined(separator: "\n") + let joined = try Self.joinedPaths(paths) return try start { bridge, error in CrocmobileStartStoreSend(joined, opts, bridge, error) } @@ -145,6 +145,16 @@ public actor CrocEngine { bridge = nil } + /// The Go side splits the list on "\n", so a path holding a line break would + /// turn into several send paths (`a.jpg\n..` sends the parent folder). + private static func joinedPaths(_ paths: [String]) throws -> String { + if paths.contains(where: { $0.contains(where: \.isNewline) }) { + throw CrocEngineError.startFailed( + EngineFailure(code: .invalidSetting, message: "a file name contains a line break")) + } + return paths.joined(separator: "\n") + } + private static func storeOptions(proxy: ProxySettings) -> CrocmobileStoreOptions { let opts = CrocmobileStoreOptions() opts.socks5Proxy = proxy.socks5 diff --git a/CrocKit/Tests/CrocKitTests/SendPathValidationTests.swift b/CrocKit/Tests/CrocKitTests/SendPathValidationTests.swift new file mode 100644 index 00000000..2453aff3 --- /dev/null +++ b/CrocKit/Tests/CrocKitTests/SendPathValidationTests.swift @@ -0,0 +1,37 @@ +import Foundation +import Testing + +@testable import CrocKit + +/// The Go side splits the joined path list on "\n", so a path carrying a line +/// break would become several send paths ("a.jpg\n.." sends the parent folder). +@Suite struct SendPathValidationTests { + private func isRefused(_ start: () async throws -> EngineTransfer) async -> Bool { + do { + _ = try await start() + return false + } catch CrocEngineError.startFailed(let failure) { + return failure.code == .invalidSetting + } catch { + return false + } + } + + @Test(arguments: ["photo.jpg\n..", "a\rb", "/tmp/a\r\nb", "\n"]) + func sendRefusesLineBreakInPath(path: String) async { + let engine = CrocEngine() + let refused = await isRefused { + try await engine.startSend(paths: ["/tmp/ok.txt", path], text: nil, options: .init()) + } + #expect(refused) + } + + @Test(arguments: ["photo.jpg\n..", "a\rb", "/tmp/a\r\nb", "\n"]) + func storeSendRefusesLineBreakInPath(path: String) async { + let engine = CrocEngine() + let refused = await isRefused { + try await engine.startStoreSend(paths: [path], options: StoredSendOptions()) + } + #expect(refused) + } +} diff --git a/app/CrocApp/Support/ReceivedName.swift b/app/CrocApp/Support/ReceivedName.swift index ad8a002b..602a5c53 100644 --- a/app/CrocApp/Support/ReceivedName.swift +++ b/app/CrocApp/Support/ReceivedName.swift @@ -2,13 +2,16 @@ import Foundation enum ReceivedName { /// True when a name from an incoming file list must block the transfer: - /// absolute path, parent-directory traversal, backslash, or NUL byte. + /// absolute path, parent-directory traversal, backslash, or a control character (NUL, line breaks, tab, DEL). /// Plain relative subpaths ("folder/file.txt") are legitimate for /// folder transfers and stay allowed. static func isUnsafe(_ name: String) -> Bool { if name.isEmpty || name.hasPrefix("/") || name.contains("\\") || name.contains("\0") { return true } + if name.unicodeScalars.contains(where: { $0.properties.generalCategory == .control }) { + return true + } return name.split(separator: "/", omittingEmptySubsequences: false).contains("..") } } diff --git a/app/CrocAppTests/ReceivedNameTests.swift b/app/CrocAppTests/ReceivedNameTests.swift index f7174da0..cb1ca5b3 100644 --- a/app/CrocAppTests/ReceivedNameTests.swift +++ b/app/CrocAppTests/ReceivedNameTests.swift @@ -13,6 +13,7 @@ import Testing @Test(arguments: [ "", "/etc/passwd", "..", "../x", "a/../b", "a/..", "a\\b", "..\\x", "a\0b", + "photo.jpg\n..", "a\rb", "a\tb", "a\u{7F}b", "a\u{85}b", ]) func blocksUnsafeNames(name: String) { #expect(ReceivedName.isUnsafe(name)) diff --git a/app/CrocShare/ShareStager.swift b/app/CrocShare/ShareStager.swift index 79e43a84..d094a422 100644 --- a/app/CrocShare/ShareStager.swift +++ b/app/CrocShare/ShareStager.swift @@ -77,7 +77,10 @@ enum ShareStager { return } var name = url.lastPathComponent - if name.isEmpty || name.hasPrefix(".") { name = "shared-file" } + let hasControl = name.unicodeScalars.contains { + $0.properties.generalCategory == .control + } + if name.isEmpty || name.hasPrefix(".") || hasControl { name = "shared-file" } var candidate = name var counter = 2 while taken.contains(candidate) diff --git a/crocmobile/crocmobile.go b/crocmobile/crocmobile.go index 5967d564..c3a3fa4a 100644 --- a/crocmobile/crocmobile.go +++ b/crocmobile/crocmobile.go @@ -159,12 +159,7 @@ func StartSend(pathsJoined string, text string, opts *Options, d Delegate) (t *T if d == nil { return nil, startError(errors.New("delegate required")) } - var paths []string - for _, p := range strings.Split(pathsJoined, "\n") { - if p = strings.TrimSpace(p); p != "" { - paths = append(paths, p) - } - } + paths := splitPaths(pathsJoined) if text == "" && len(paths) == 0 { return nil, startError(errors.New("nothing to send")) } @@ -199,3 +194,15 @@ func StartReceive(code string, opts *Options, d Delegate) (t *Transfer, err erro } return &Transfer{s: s}, nil } + +// splitPaths splits a "\n"-joined path list without trimming: a file name may +// begin or end with a space. +func splitPaths(joined string) []string { + var paths []string + for _, p := range strings.Split(joined, "\n") { + if p != "" { + paths = append(paths, p) + } + } + return paths +} diff --git a/crocmobile/session.go b/crocmobile/session.go index 704740f0..b81f95af 100644 --- a/crocmobile/session.go +++ b/crocmobile/session.go @@ -129,6 +129,14 @@ const ( // which startReceiveSession relies on for AutoAccept. const promptPolicyAnswers = 4096 +// keptPromptAnswers is the only batch ever queued behind the accept answer, +// whatever the policy: "n" keeps what exists (the unzip skips that member), so +// Replace overwrites files through croc's Overwrite but never a zip member. +// Never "y": croc v11 re-runs the accept prompt on the sender's new file list +// after each reconnect, and that prompt would read a queued "y" as consent to +// a list the user never saw. +var keptPromptAnswers = strings.Repeat("n\n", promptPolicyAnswers) + func parsePolicy(s string) filePolicy { if s == PolicyReplace { return replace @@ -148,16 +156,32 @@ func (p filePolicy) apply(co *croc.Options) { co.Overwrite = p == replace } -// promptAnswers is the batch of answers croc's per-file prompts read: "y" -// replaces, "n" keeps what exists (the unzip skips that member). -func (p filePolicy) promptAnswers() string { - answer := "n\n" - if p == replace { - answer = "y\n" +// maxReportedBytes caps every size and byte sum the delegate sees (1 PiB). A +// file list's sizes are the sender's word, and unchecked sums of them overflow +// int64 here and trap in the app's Int64 arithmetic. +const maxReportedBytes = 1 << 50 + +func addReported(total, size int64) int64 { + return min(total+min(max(size, 0), maxReportedBytes), maxReportedBytes) +} + +// sendFilesInfo hands croc the picked paths as literal names. croc expands any +// path holding '*' with filepath.Glob, so a picked file named "*.pdf" would +// also send every PDF beside it; escaping the pattern makes it match only +// itself. +func sendFilesInfo(paths []string, o *Options) ([]croc.FileInfo, []croc.FileInfo, int, error) { + literal := make([]string, len(paths)) + for i, p := range paths { + if strings.Contains(p, "*") { + p = globEscaper.Replace(p) + } + literal[i] = p } - return strings.Repeat(answer, promptPolicyAnswers) + return croc.GetFilesInfoWithExactExclusions(literal, o.ZipFolder, o.GitIgnore, splitNonEmpty(o.Exclude, "\n"), nil) } +var globEscaper = strings.NewReplacer(`\`, `\\`, `*`, `\*`, `?`, `\?`, `[`, `\[`) + func splitNonEmpty(s, sep string) []string { var out []string for _, p := range strings.Split(s, sep) { @@ -250,8 +274,7 @@ func startSession(sender bool, code string, paths []string, text string, o *Opti } paths = []string{f.Name()} } - filesInfo, emptyFolders, totalFolders, err := croc.GetFilesInfoWithExactExclusions( - paths, o.ZipFolder, o.GitIgnore, splitNonEmpty(o.Exclude, "\n"), nil) + filesInfo, emptyFolders, totalFolders, err := sendFilesInfo(paths, o) if err != nil { if tempPath != "" { os.Remove(tempPath) @@ -551,7 +574,7 @@ func (s *session) respond(accept bool, policy filePolicy) { if s.client != nil { policy.apply(&s.client.Options) } - answers += policy.promptAnswers() + answers += keptPromptAnswers } _, _ = s.promptW.WriteString(answers) } else { @@ -564,18 +587,16 @@ func (s *session) respond(accept bool, policy filePolicy) { } // answerUngatedPrompts queues answers for an AutoAccept receive, which has no -// accept prompt, and closes the pipe. Always Keep Both's "n", whatever the -// policy: croc still forces the accept prompt when the remote sender ran -// --ask, that prompt reads the first queued line, and "y" would accept a -// transfer the sender wanted confirmed by hand. So under AutoAccept, Replace -// overwrites files through croc's Overwrite but keeps existing zip members. +// accept prompt, and closes the pipe. croc still forces the accept prompt when +// the remote sender ran --ask, and that prompt reads the first queued line, so +// it must be "n" here too. func (s *session) answerUngatedPrompts() { s.promptM.Lock() defer s.promptM.Unlock() if s.promptW == nil { return } - _, _ = s.promptW.WriteString(keepBoth.promptAnswers()) + _, _ = s.promptW.WriteString(keptPromptAnswers) s.promptW.Close() s.promptW = nil } @@ -727,8 +748,8 @@ func (s *session) fileListJSON() string { var total int64 for _, f := range c.FilesToTransfer { name := filepath.Join(f.FolderRemote, f.Name) - files = append(files, fileEntry{Name: name, Size: f.Size}) - total += f.Size + files = append(files, fileEntry{Name: name, Size: addReported(0, f.Size)}) + total = addReported(total, f.Size) } b, _ := json.Marshal(map[string]any{ "files": files, "emptyFolders": len(c.EmptyFoldersToTransfer), "totalSize": total, @@ -763,17 +784,17 @@ func (s *session) progressSnapshot(connected bool) progressState { // goroutines, and a concurrent read here races it -- a fatal, unrecoverable // runtime throw, not a panic recover() can catch. for i, f := range c.FilesToTransfer { - totalSize += f.Size + totalSize = addReported(totalSize, f.Size) if i < cur { - bytesFinished += f.Size + bytesFinished = addReported(bytesFinished, f.Size) } } if cur >= 0 && cur < len(c.FilesToTransfer) { f := c.FilesToTransfer[cur] fileName = f.Name - fileSize = f.Size + fileSize = addReported(0, f.Size) } - sent := c.TotalSent // per-current-file + sent := addReported(0, c.TotalSent) // per-current-file if c.Step3RecipientRequestFile || sent > 0 { step = "transferring" } @@ -788,7 +809,7 @@ func (s *session) summaryJSON() string { c := s.client var total int64 for _, f := range c.FilesToTransfer { - total += f.Size + total = addReported(total, f.Size) } b, _ := json.Marshal(map[string]any{ "success": c.SuccessfulTransfer, "files": len(c.FilesToTransfer), "totalSize": total, diff --git a/crocmobile/session_test.go b/crocmobile/session_test.go index 4c9f0e82..2d179f34 100644 --- a/crocmobile/session_test.go +++ b/crocmobile/session_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "io" + "math" "os" "path/filepath" "reflect" @@ -269,6 +270,86 @@ func TestProgressSnapshot(t *testing.T) { } } +// Sizes in a file list are the sender's word: a peer can declare sizes whose +// sums overflow int64, so every reported size and sum is capped. +func TestProgressSnapshotCapsDeclaredSizes(t *testing.T) { + s := &session{client: &croc.Client{ + FilesToTransfer: []croc.FileInfo{ + {Name: "x", Size: math.MaxInt64 - 10}, + {Name: "y", Size: math.MaxInt64 - 10}, + {Name: "z", Size: -5}, + }, + FilesToTransferCurrentNum: 2, + TotalSent: math.MaxInt64, + }} + want := progressState{ + CurrentFile: 2, TotalFiles: 3, FileName: "z", FileSent: maxReportedBytes, + BytesFinished: maxReportedBytes, TotalSize: maxReportedBytes, Step: "transferring", + } + if got := s.progressSnapshot(true); got != want { + t.Fatalf("progressSnapshot\n got: %+v\nwant: %+v", got, want) + } +} + +func TestFileListJSONCapsDeclaredSizes(t *testing.T) { + s := &session{client: &croc.Client{FilesToTransfer: []croc.FileInfo{ + {Name: "x", FolderRemote: ".", Size: math.MaxInt64}, + {Name: "y", FolderRemote: ".", Size: math.MaxInt64}, + {Name: "z", FolderRemote: ".", Size: -1}, + }}} + var got struct { + Files []fileEntry `json:"files"` + TotalSize int64 `json:"totalSize"` + } + if err := json.Unmarshal([]byte(s.fileListJSON()), &got); err != nil { + t.Fatal(err) + } + if got.TotalSize != maxReportedBytes { + t.Errorf("totalSize = %d, want %d", got.TotalSize, int64(maxReportedBytes)) + } + for i, want := range []int64{maxReportedBytes, maxReportedBytes, 0} { + if got.Files[i].Size != want { + t.Errorf("files[%d].size = %d, want %d", i, got.Files[i].Size, want) + } + } +} + +// A picked file whose name holds '*' is sent alone: croc globs any path with a +// '*', which would add every matching file next to it. +func TestSendFilesInfoTakesPathsLiterally(t *testing.T) { + dir := t.TempDir() + for _, name := range []string{"*.pdf", "a.pdf", "b.pdf", "[x]*", "[x]y", `a\*`} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(name), 0o600); err != nil { + t.Fatal(err) + } + } + for _, name := range []string{"*.pdf", "[x]*", `a\*`} { + files, _, _, err := sendFilesInfo([]string{filepath.Join(dir, name)}, &Options{}) + if err != nil { + t.Fatalf("%q: %v", name, err) + } + if len(files) != 1 || files[0].Name != name { + var got []string + for _, f := range files { + got = append(got, f.Name) + } + t.Errorf("%q: sent %q, want only that file", name, got) + } + } +} + +// Path lists cross the bridge joined by "\n". A picked name that ends in a +// space must stay that name, or the send picks up the sibling without it. +func TestSplitPathsKeepsNamesExact(t *testing.T) { + got := splitPaths("/d/report.pdf \n\n /d/x\n") + if want := []string{"/d/report.pdf ", " /d/x"}; !reflect.DeepEqual(got, want) { + t.Fatalf("splitPaths = %q, want %q", got, want) + } + if got := splitPaths(""); got != nil { + t.Fatalf("splitPaths(\"\") = %q, want nil", got) + } +} + func keysOf(m map[string]any) []string { keys := make([]string, 0, len(m)) for k := range m { @@ -405,7 +486,6 @@ func TestSummaryJSON(t *testing.T) { func TestRespond(t *testing.T) { keep := strings.Repeat("n\n", promptPolicyAnswers) - repl := strings.Repeat("y\n", promptPolicyAnswers) tests := []struct { name string sender bool @@ -415,7 +495,7 @@ func TestRespond(t *testing.T) { want string }{ {"receive accept keeps both", false, true, keepBoth, 0, "y\n" + keep}, - {"receive accept replaces", false, true, replace, 0, "y\n" + repl}, + {"receive accept replace queues no y", false, true, replace, 0, "y\n" + keep}, {"receive decline", false, false, replace, 0, "n\n"}, {"send accept zero value counts as one", true, true, keepBoth, 0, "y\n"}, {"send accept negative counts as one", true, true, keepBoth, -2, "y\n"}, @@ -606,7 +686,7 @@ func TestPolicyAnswersFitPipeBuffer(t *testing.T) { } defer pr.Close() defer pw.Close() - batch := "y\n" + replace.promptAnswers() + batch := "y\n" + keptPromptAnswers if err := pw.SetWriteDeadline(time.Now().Add(time.Second)); err != nil { t.Fatal(err) } @@ -623,7 +703,7 @@ func TestDrainPromptInputClearsLeftoverAnswers(t *testing.T) { if err != nil { t.Fatal(err) } - _, _ = first.promptW.WriteString("y\n" + keepBoth.promptAnswers()) + _, _ = first.promptW.WriteString("y\n" + keptPromptAnswers) ctx := context.Background() if got, err := utils.GetInputContext(ctx, ""); err != nil || got != "y" { t.Fatalf("first prompt read %q, %v; want y", got, err) diff --git a/crocmobile/store.go b/crocmobile/store.go index 3b8f60f7..a8786854 100644 --- a/crocmobile/store.go +++ b/crocmobile/store.go @@ -110,7 +110,7 @@ func StartStoreSend(pathsJoined string, o *StoreOptions, d Delegate) (t *Transfe if d == nil { return nil, startError(errors.New("delegate required")) } - paths := splitNonEmpty(pathsJoined, "\n") + paths := splitPaths(pathsJoined) if err := checkStoreLimits(paths); err != nil { return nil, startError(err) } diff --git a/docs/knowledge/app-ui-architecture.md b/docs/knowledge/app-ui-architecture.md index 68be1930..c70346dd 100644 --- a/docs/knowledge/app-ui-architecture.md +++ b/docs/knowledge/app-ui-architecture.md @@ -33,7 +33,7 @@ Construction order in `CrocAppApp.init` matters: `AppSettings` first, then `Tran - **`transferActive` on start: retry once after ~300 ms.** There is a release window after `done` where the next start still throws. - `handle(.failed)` does not call the engine: the stream's `onTermination` cancels and clears the transfer by its ID. `TransferController` keeps that `transferID`; every cancel (user, background expiry, auto-accept block) targets it, and a cancel tapped before `run()` has the ID is applied as soon as `startSend`/`startReceive` returns. - Copy precedence when several conditions collide, highest first: `blockedAutoAccept`, `backgroundExpired`, the auto-accept-vs-remote-`--ask` case, then cancel/decline mapping. `run()`'s catch mirrors the `backgroundExpired` step so a thrown expiry keeps the resume hint. -- **Code `refused` under auto-accept is not a peer decline.** croc forces the receiver prompt whenever the *remote sender* ran `--ask` (`croc.go`: `!NoPrompt || Ask || senderInfo.Ask`), and engine auto-accept has already closed the prompt pipe, so `GetInput` hits EOF and croc reports its local `refused files`. `autoAcceptActive && !cancelRequested && failure.code == .refused` discriminates it (after the `blockedAutoAccept` and `backgroundExpired` branches): `respond()` is never called on the auto-accept path, so `declineRequested` cannot be set. Under AutoAccept the pipe is not empty: crocmobile queues a batch of `n` answers for croc's other un-gated prompts (empty-folder, unzip member) and then closes it (`crocmobile-bridge.md`). The forced remote-`--ask` prompt reads the first `n` and ends as `refused` under either policy; the batch is `n` even for Replace for exactly this reason (`verify-interop.sh` scenario 12 checks it). +- **Code `refused` under auto-accept is not a peer decline.** croc forces the receiver prompt whenever the *remote sender* ran `--ask` (`croc.go`: `!NoPrompt || Ask || senderInfo.Ask`), and engine auto-accept has already closed the prompt pipe, so `GetInput` hits EOF and croc reports its local `refused files`. `autoAcceptActive && !cancelRequested && failure.code == .refused` discriminates it (after the `blockedAutoAccept` and `backgroundExpired` branches): `respond()` is never called on the auto-accept path, so `declineRequested` cannot be set. Under AutoAccept the pipe is not empty: crocmobile queues a batch of `n` answers for croc's other un-gated prompts (empty-folder, unzip member) and then closes it (`crocmobile-bridge.md`). The forced remote-`--ask` prompt reads the first `n` and ends as `refused` under either policy; the batch is `n` even for Replace for exactly this reason, and after a manual accept too, where a croc v11 reconnect re-runs the accept prompt (`verify-interop.sh` scenario 12 checks it). - `blockedAutoAccept` is checked in `.done` as well as `.failed`: `cancel()` is fire-and-forget, so a small transfer can finish before the cancellation reaches the engine. Both paths record `.failed` (the schema has no blocked case, and "Cancelled" reads as a user action). - `TransferStatusView` shows a "taking longer than usual" hint after 25 s (`slowPhaseThreshold`) spent in `.starting`/`.connecting`. A `.task(id: isInEarlyPhase)` resets the timer whenever that flips, so the hint never keeps counting once a transfer moves on, but it does survive the `.starting → .connecting` handoff — same stall to the user, no reason to reset the clock. diff --git a/docs/knowledge/crocmobile-bridge.md b/docs/knowledge/crocmobile-bridge.md index 7d7b8921..2c415133 100644 --- a/docs/knowledge/crocmobile-bridge.md +++ b/docs/knowledge/crocmobile-bridge.md @@ -37,7 +37,7 @@ CrocKit Swift package (CrocEngine actor, AsyncStream) → app - Receiver MUST set `outDir`; sender sets `workDir` (writable, for text/zip temp files; iOS cwd is `/`). - Abandoning the event stream cancels the Go session (`onTermination`); `Cancel()` cancels the context and also closes the prompt pipe: after a context cancel, `GetInputContext` returns but its read goroutine stays blocked until the pipe closes, so skipping the close would leave it hanging. - `Options.Ask` is exposed: sender sessions with Ask install the same dup2 fd0 pipe as receive (sender-only helper), and `Respond(true)` writes one `y\n` PER FILE (`promptAnswers = len(filesInfo)`) — croc's sender prompt fires per `TypeRecipientReady`, once per file; a single answer starves multi-file sends at file 2 (EOF ⇒ "refusing files"). Decline/cancel close the pipe; EOF at the Ask prompt refuses safely (croc checks the input error before its default-yes). -- File policy (`filePolicy`, `session.go`): `keepBoth` or `replace`, what a receive does with a file whose name is taken. `buildCrocOptions` always hands croc exactly one of `Rename` and `Overwrite`: neither set, or both set, means keep both (Rename wins); an unknown policy string also means keep both. croc's third state (both false) is never passed: it prompts `Overwrite ...? (y/N)` on stdin, which on our closed pipe skips the file, and on a zipped-folder unzip aborts the transfer. On accept, `respond` sets the policy on the live `croc.Client` before writing `y` (croc reads `Rename`/`Overwrite` only once the accept prompt is answered), then queues 4096 more answers (`promptPolicyAnswers`) for the stdin prompts that ignore both flags: the empty-folder prompt and the per-member unzip prompt `Overwrite ''? (y/N)`. Replace answers `y`; Keep Both answers `n`, which makes the unzip skip that existing member and keep the local copy. 4096 two-byte lines fit a fresh pipe buffer on Darwin and Linux, so the write never blocks. With `AutoAccept` there is no accept prompt: the batch is written at session start and the pipe is closed (`answerUngatedPrompts`). That batch is always Keep Both's `n`, whatever the policy: croc still forces the accept prompt when the remote sender ran `--ask`, the prompt reads the first queued line, and `y` would accept a transfer the sender wanted confirmed by hand (no line means "no" to the accept prompt and "yes" to the unzip prompt). So under AutoAccept, Replace overwrites files through `Overwrite` but keeps existing zip members. Past 4096 colliding members an unzip hits EOF and fails, leaving the received zip (`docs/known-issues.md`, Accepted). A file that already exists with identical content is left as is under either policy (croc counts it unchanged). Keep Both names come from `utils.UnusedFilename`: `name (1).ext`, `name (2).ext`. +- File policy (`filePolicy`, `session.go`): `keepBoth` or `replace`, what a receive does with a file whose name is taken. `buildCrocOptions` always hands croc exactly one of `Rename` and `Overwrite`: neither set, or both set, means keep both (Rename wins); an unknown policy string also means keep both. croc's third state (both false) is never passed: it prompts `Overwrite ...? (y/N)` on stdin, which on our closed pipe skips the file, and on a zipped-folder unzip aborts the transfer. On accept, `respond` sets the policy on the live `croc.Client` before writing `y` (croc reads `Rename`/`Overwrite` only once the accept prompt is answered), then queues 4096 more answers (`promptPolicyAnswers`) for the stdin prompts that ignore both flags: the empty-folder prompt and the per-member unzip prompt `Overwrite ''? (y/N)`. Every queued answer is `n` under either policy (`keptPromptAnswers`), which makes the unzip skip an existing member and keep the local copy, so Replace overwrites files through `Overwrite` but never a zip member. Never `y`: croc v11 re-runs the accept prompt on the sender's new file list after each reconnect, and a queued `y` would accept a list the user never saw (security scan F2, 2026-10-15). The same `n` therefore refuses any reconnect after a manual accept (`docs/known-issues.md`, Accepted). 4096 two-byte lines fit a fresh pipe buffer on Darwin and Linux, so the write never blocks. With `AutoAccept` there is no accept prompt: the batch is written at session start and the pipe is closed (`answerUngatedPrompts`). That batch is the same `n` batch: croc still forces the accept prompt when the remote sender ran `--ask`, the prompt reads the first queued line, and `y` would accept a transfer the sender wanted confirmed by hand (no line means "no" to the accept prompt and "yes" to the unzip prompt). Past 4096 colliding members an unzip hits EOF and fails, leaving the received zip (`docs/known-issues.md`, Accepted). A file that already exists with identical content is left as is under either policy (croc counts it unchanged). Keep Both names come from `utils.UnusedFilename`: `name (1).ext`, `name (2).ext`. - `Quiet` stays false (true replaces `os.Stderr` process-wide with /dev/null); croc progress bar noise on stderr is accepted. - Go `crocmobile.Options` exports more knobs than Swift wires: `RelayPorts`, `Curve`, `HashAlgorithm`, `ThrottleUpload`, `NoMultiplexing` are settable from Go/`croctest` only — `EngineOptions`/`crocOptions(from:)` never set them. Exposing them in the GUI means plumbing CrocKit first. `Options.Rename` exists next to `Overwrite` but Swift never sets either: `EngineOptions.whenFileExists` (`FilePolicy`, default `.keepBoth`) is the one knob, see the file policy item under Engine behavior invariants. - Transport: `Options.Transport` is croc v11's sender-only `Transport`. `TransportAuto` (`"auto"`, also the empty string) and `TransportRelay` (`"relay"`) are the values the app uses; Swift mirrors them as the `Transport` enum (`auto`, `relay`) on `EngineOptions.transport`, stored as `AppSettings.transport` (`settings.transport`, an unknown stored value reads as auto). `sessionTransport` (`session.go`) forces auto on a receive and on a local-only send, because croc rejects anything else there (`transport selection is sender-only`, and `--transport` with `--local` unless auto), and passes any other value through croc's `ParseTransportMode`, so an unknown value is a start error. croc's `derp` (Tailcat only, no relay fallback) parses and reaches croc, but only `croctest -transport auto|relay|derp` and Go callers can set it; the app never offers it. The Settings picker is disabled while Local network only is on. See "croc v11.5.4 gotchas" for what auto does. diff --git a/docs/known-issues.md b/docs/known-issues.md index 94440099..746bb030 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -62,7 +62,9 @@ Engine and bridge invariants live in `knowledge/crocmobile-bridge.md`; UI state- - **`dropDestination(for:action:isTargeted:)` stays at `ContentView.swift` and `SendView.swift`.** The Xcode 27 SDK marks it deprecated only for a future OS (`deprecated: 100000.0`), so builds emit no warning. The replacement `dropDestination(for:isEnabled:action:)` takes a `Void` action (a drop cannot be rejected) and has no `isTargeted`; its highlight API `onDropSessionUpdated` is iOS 27 / macOS 26 only, and the deployment target is iOS 26.0. Revisit when the minimum OS reaches 27. - **`OutputFolderStore.select` silently no-ops if bookmarking fails.** Rare, and the alternative is an error path for a condition the user cannot act on. - **A zipped-folder receive that collides with more than 4096 existing members still fails.** crocmobile answers croc's per-member `Overwrite ''? (y/N)` prompts from a batch of 4096 lines written up front (`promptPolicyAnswers`), sized to fit a fresh pipe buffer on Darwin and Linux so the write never blocks. Past that, croc's unzip reads EOF and fails with `received archive failed validation or extraction`, leaving the received `.zip`. Accepted: a larger batch could block the write (`knowledge/crocmobile-bridge.md`). -- **The incoming sheet cannot preview which members of a zipped folder already exist.** The file list shows one `.zip` name, so the conflict scan sees none of its members and offers only Accept (Keep Both). Keep Both then silently keeps the existing members and skips the matching ones in the archive. Auto-accept keeps them too, even set to Replace: its stdin answers are written before croc runs, and the first one also answers the accept prompt croc forces when the sender ran `--ask`, so it is always "n" (`knowledge/crocmobile-bridge.md`). Replace reaches zip members only when another file in the same transfer collides, so the sheet offers it. Accepted: the archive's contents are only known to croc after it unpacks, and keeping what is there loses nothing. +- **The incoming sheet cannot preview which members of a zipped folder already exist.** The file list shows one `.zip` name, so the conflict scan sees none of its members and offers only Accept (Keep Both). The existing members are kept and the matching ones in the archive skipped under every policy, Replace and auto-accept included: the stdin answers croc's unzip prompt reads are always "n", because a queued "y" would also answer the accept prompt croc re-runs after a reconnect or forces when the sender ran `--ask` (`knowledge/crocmobile-bridge.md`). Accepted: the archive's contents are only known to croc after it unpacks, and keeping what is there loses nothing. +- **A file whose name ends in a space cannot be sent to a croc v11 peer.** CrocApp sends the exact name (crocmobile `splitPaths`, B22), but croc 11.5.4 receivers, CLI and crocmobile alike, refuse it: `unsafe receive path in "report.pdf ": Windows-trimmed trailing character`; the sender sees `cancelled`. Accepted: the refusal is croc's receive-path policy, and before B22 the send silently picked up the sibling file without the space. Rename the file to send it. +- **A croc v11 reconnect after a manually accepted receive ends as `refused`.** croc re-runs the accept prompt on the sender's file list after each reconnect (`resetForReconnectAttempt`, then `processMessageFileInfo`), and the next queued stdin answer is "n", so the receive fails instead of resuming. Accepted: answering "y" there would accept a list the user never saw (security scan F2, 2026-10-15), and crocmobile cannot tell that prompt from the unzip prompt on the same pipe. From reading croc v11.5.4; no harness forces a reconnect. - **The trust badge names the configured route, not the path that carried the data.** `TrustBadge` reads the relay setting, so a transfer that croc moved over the LAN, or over Tailcat under Connection = Automatic, still shows "Via the public croc relay" (or the custom relay). croc does not report the winning path to the engine. Accepted: the badge's claim that the server sees only encrypted data holds on every path. - **Receive list identity is by file name.** Duplicate names within one transfer are a croc-level concern, not a UI one. - **Gesture-level flows are not machine-verified.** Drag-and-drop, camera QR, and taps have no XCUITest coverage, by decision: unit tests cover non-UI logic (Go `go test -race`, Swift Testing in `CrocKitTests` and `CrocAppTests`, all in CI), transfers are covered by the harnesses, and UI flows are verified by hand. The rule is in `CONTRIBUTING.md` (Verification, Tests). diff --git a/scripts/verify-interop.sh b/scripts/verify-interop.sh index 7a323237..add0df18 100755 --- a/scripts/verify-interop.sh +++ b/scripts/verify-interop.sh @@ -226,11 +226,11 @@ pass "v10 receiver: peerVersion" # prompt and preset with -yes. Keep Both leaves the old file and adds # "a (1).txt"; Replace leaves one file with the new content. A zipped folder # received twice into one folder hits croc's per-member unzip prompt, which -# reads the answers crocmobile queues: Keep Both skips the existing member, -# Replace at the prompt overwrites it, and both finish without an error. Under -# -yes the queued answers are always "n", so that a sender's --ask prompt, -# which croc forces even under -yes, is still refused with Replace: the last -# case checks that, and the zipped folder keeps its member. +# reads the answers crocmobile queues. They are always "n", so every policy +# keeps the existing member and finishes without an error: a queued "y" would +# also answer the accept prompt croc re-runs after a reconnect, and the one +# croc forces under -yes when the sender ran --ask. The last case checks that +# --ask prompt is refused with Replace. SCENARIO=12 collide() { # collide NAME POLICY ACCEPT... local name=$1 policy=$2 dir="$TMP/dst12-$1" C @@ -270,11 +270,7 @@ zip_twice() { # zip_twice NAME POLICY ACCE [ ! -e "$dir/sub.zip" ] || fail "zipped folder $policy ($*), round $round: archive left behind" [ "$round" = 2 ] || echo "local edit" > "$dir/sub/b.txt" done - if [ "$policy" = keepBoth ] || [ "$1" = -yes ]; then - [ "$(cat "$dir/sub/b.txt")" = "local edit" ] || fail "zipped folder keepBoth ($*): existing member changed" - else - diff "$TMP/src/sub/b.txt" "$dir/sub/b.txt" > /dev/null || fail "zipped folder replace ($*): member not replaced" - fi + [ "$(cat "$dir/sub/b.txt")" = "local edit" ] || fail "zipped folder $policy ($*): existing member changed" pass "zipped folder received twice, $policy ($*)" } zip_twice kp keepBoth -answer y diff --git a/web/docs/content/guide/receiving.md b/web/docs/content/guide/receiving.md index 94faf19f..f9c8e625 100644 --- a/web/docs/content/guide/receiving.md +++ b/web/docs/content/guide/receiving.md @@ -47,11 +47,11 @@ CrocApp never overwrites an existing file unless you say so. If any incoming fil A file that already exists with identical content is left as it is under either choice. Files with unsafe names (for example, ones trying to escape the destination folder) block the accept buttons entirely, as a safety measure on top of croc's own name sanitization. -A folder sent as a zip arrives as a single `.zip` name, so the preview cannot see which of its files already exist. The sheet then offers only **Accept**, which is Keep Both: it silently keeps the existing files and skips the matching ones in the archive. Auto-accept keeps them too, even when set to Replace. To get the sender's copies, move the old folder away before receiving. +A folder sent as a zip arrives as a single `.zip` name, so the preview cannot see which of its files already exist. The sheet then offers only **Accept**, which is Keep Both: it silently keeps the existing files and skips the matching ones in the archive. Replace and auto-accept keep them too. To get the sender's copies, move the old folder away before receiving. ## Auto-accept (F18) -Auto-accept is **off by default**. When off, you always see the file-list preview above before anything is saved. Turning it on (in [power options](power-options.md)) skips that preview — see that page for the exact warning CrocApp shows when it's enabled. Because there is no sheet to ask, a setting under it, **When a file already exists**, decides between Keep Both (the default) and Replace. Replace overwrites existing files, except inside a zipped folder: there the existing files are kept, because answering that unzip prompt with yes would also accept a transfer whose sender asked you to confirm it by hand. +Auto-accept is **off by default**. When off, you always see the file-list preview above before anything is saved. Turning it on (in [power options](power-options.md)) skips that preview — see that page for the exact warning CrocApp shows when it's enabled. Because there is no sheet to ask, a setting under it, **When a file already exists**, decides between Keep Both (the default) and Replace. Replace overwrites existing files, except inside a zipped folder, where the existing files are kept (see above). ## After the transfer From b387c6c0a731f408edee4b4ef757e49157eb7c53 Mon Sep 17 00:00:00 2001 From: Bakir Gracic Date: Thu, 15 Oct 2026 06:32:50 +0200 Subject: [PATCH 3/4] Review the web tree for the security requirement and record its findings The scan of a7a87cf left out web/. A read-only review found no injection sink and no High finding. The receive-link page no longer calls the code encrypted, and both docs pages say the browser path sends it to getcroc.com in a query string. The other findings are in known-issues.md. --- docs/known-issues.md | 5 +++++ web/docs/content/guide/code-phrases.md | 2 +- web/docs/content/security-and-privacy.md | 2 +- web/landing/r/index.html | 4 ++-- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/known-issues.md b/docs/known-issues.md index 746bb030..edcd727e 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -46,6 +46,9 @@ Engine and bridge invariants live in `knowledge/crocmobile-bridge.md`; UI state- - **`--content-max-width` on the hero mockup.** `design/web.md` now carves an explicit exception for elements that depict the app, so this is documented rather than fixed — but the exception is narrow and easy to over-apply. - **Store badges are inverted in dark mode with a CSS filter.** Apple ships black and white variants; the repo has black only. Inverting a black-on-transparent badge produces the white variant, but it is a filter over supplied artwork, not the supplied artwork. - **`llms.txt`'s release status is hand-maintained and nothing checks it.** It has to be edited by hand on every release. The JSON-LD `dateModified` used to sit beside it and has been dropped: it rotted on every commit that touched the page, templating it from `git log` puts a moving part inside a structured-data block where a broken template corrupts silently rather than failing loudly, and `datePublished` alone is true forever. `og.jpg` is a second hand-made asset but a deliberate one, fixed rather than rotting. +- **The landing and docs pages set no Content-Security-Policy.** GitHub Pages cannot send headers, so only a `` CSP is possible, and it needs a hash for each inline script (the theme script on every page, the code parser in `r/index.html`, Docusaurus's own inline scripts) that must be regenerated on every edit. No injection sink exists today: `r/index.html` and `app.js` write with `textContent` and build links with `encodeURIComponent`, and nothing loads third-party script (`web/` review, 2026-10-15, W1). Recorded rather than fixed for 1.0: a wrong hash breaks the page silently in the browser, and `scripts/assemble-site.sh` does not yet check it. Clickjacking of the `/r/` buttons stays open either way, since `frame-ancestors` is ignored in a meta tag. +- **`/r/` accepts any fragment of six or more characters without whitespace as a code**, so `/r/#download` offers to open "download" in CrocApp. Display only; the text is escaped and the app validates the code (W5). +- **The AASA file claims `/r/*`**, wider than the one page that exists. `/r/x` is a Pages 404, so no real link is taken (W6). `security.txt` expires 2027-08-01 and needs renewing before then (W7). ## Docs site @@ -54,6 +57,8 @@ Engine and bridge invariants live in `knowledge/crocmobile-bridge.md`; UI state- - **A landing-only change rebuilds the docs site too, and vice versa.** `github-pages.yml` calls `scripts/assemble-site.sh`, which builds both surfaces regardless of which one changed. - **Eleven of the eighteen screenshot pairs are unused.** `scripts/capture-screenshots.sh` writes eighteen light/dark pairs; the README, six docs pages and the landing hero between them reference seven. The rest are deliberate headroom, not waste — adding a usage is cheap, re-capturing a deleted one is not. - **The two sitemaps stay separate.** `web/landing/sitemap.xml` is hand-maintained and lists the one landing URL; `robots.txt` advertises the Docusaurus-generated `/docs/sitemap.xml` alongside it, so crawlers reach both. Not a defect with a single landing route — the `lastmod` that used to rot there has been dropped. +- **The docs build is not fully pinned.** `packageManager` in `web/docs/package.json` has no `+sha512` hash, so the workflow's corepack fetch is not integrity-checked (`corepack use pnpm@` writes it); the four `overrides` in `pnpm-workspace.yaml` are open-ended `>=` floors; and `pnpm audit --prod` reports one High advisory with no fix, `braces@3.0.3` (GHSA-vfj7-8cjw-p6xm, stack exhaustion), reached through chokidar and micromatch inside Docusaurus core. All build-time only; no attacker input reaches the glob handling (`web/` review, 2026-10-15, W3, W4, W9). Re-run the audit on each Docusaurus upgrade. +- **The Pages "cancel" step probably does nothing.** `github-pages.yml` posts to `pages/deployments/$SHA/cancel` and ends in `|| true`; `github.sha` is a commit, not a Pages deployment ID (inference, not tested), so a cancelled run may leave the deployment hanging (W8). ## Accepted diff --git a/web/docs/content/guide/code-phrases.md b/web/docs/content/guide/code-phrases.md index 70bda32c..4cee46f0 100644 --- a/web/docs/content/guide/code-phrases.md +++ b/web/docs/content/guide/code-phrases.md @@ -46,7 +46,7 @@ Opening a link, or pasting or scanning one, takes you to **Receive** with the co The page at `crocapp.dev/r/` reads the code from the part of the address after the `#`. Browsers never send that part to a server, and the page makes no network request. From the page you can open CrocApp, get it from the App Store or as a disk image, or receive in your browser. -**Receive in your browser** opens `https://getcroc.com/?code=`, which is croc's own web app, not part of CrocApp. That path trusts getcroc.com fully, because getcroc.com serves the JavaScript that decrypts the file. It also needs the default relays, so the sender's waiting screen shows a note when a custom relay or Local network only is active. For the strongest guarantee, receive in CrocApp or the croc CLI. +**Receive in your browser** opens `https://getcroc.com/?code=`, which is croc's own web app, not part of CrocApp. The code is in the query string there, so getcroc.com's server receives it. That path trusts getcroc.com fully, because getcroc.com serves the JavaScript that decrypts the file. It also needs the default relays, so the sender's waiting screen shows a note when a custom relay or Local network only is active. For the strongest guarantee, receive in CrocApp or the croc CLI. ## Custom codes diff --git a/web/docs/content/security-and-privacy.md b/web/docs/content/security-and-privacy.md index e22a2cf1..a1ec460a 100644 --- a/web/docs/content/security-and-privacy.md +++ b/web/docs/content/security-and-privacy.md @@ -28,7 +28,7 @@ There has been **no formal third-party security audit** of croc, and CrocApp doe ## Receiving in a browser -The link page offers **Receive in your browser**, which opens `https://getcroc.com/?code=`. That is croc's own web app, not run by CrocApp, and the browser path trusts getcroc.com fully: getcroc.com serves the JavaScript that decrypts the file, so whoever controls that site could change the code that handles your data. It also reaches only croc's default relays. The CrocApp and croc CLI paths do not have this dependency. If that trust is more than you want to give, receive in CrocApp. +The link page offers **Receive in your browser**, which opens `https://getcroc.com/?code=`. That is croc's own web app, not run by CrocApp. The code travels in the query string there, so it reaches getcroc.com's server, and the browser path trusts getcroc.com fully: getcroc.com serves the JavaScript that decrypts the file, so whoever controls that site could change the code that handles your data. It also reaches only croc's default relays. The CrocApp and croc CLI paths do not have this dependency. If that trust is more than you want to give, receive in CrocApp. ## What CrocApp does not do diff --git a/web/landing/r/index.html b/web/landing/r/index.html index 1b1eef0a..d6c2b813 100644 --- a/web/landing/r/index.html +++ b/web/landing/r/index.html @@ -43,8 +43,8 @@

Someone is sending you a file