From 2e77823b22ccf2c52a4b2964f8bf3fe7cf43585b Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 21 Sep 2026 15:32:01 -0400 Subject: [PATCH] feat(reporting): share the report reason vocabulary, and sign the target `ReportRequest` has no category field -- only a free-form `description` -- so a report's reason has to be encoded into that string. Encoding it twice, once per platform, gives moderation two spellings of the same report. New `:libs:reporting` holds the six tokens and the rule that assembles them: the token on the first line, the reporter's own words from the second on. It rides the existing `SharedCore` framework, and `ReportReason.swift` re-exports the Kotlin enum rather than restating it, so the tokens are written down once for both apps. `ReportingApi` also set its auth block before the target. `authenticate()` signs `buildPartial()`, a snapshot of the builder at that moment, so every report was signed over its description alone. The auth block now goes last, which is how `BlocklistApi` has always built its requests. --- .github/workflows/shared-core-tests.yml | 4 +- kmp/shared-core/build.gradle.kts | 2 + .../Sources/SharedCoreKit/ReportReason.swift | 29 ++++++++ .../ReportReasonTests.swift | 29 ++++++++ libs/reporting/.gitignore | 1 + libs/reporting/build.gradle.kts | 34 ++++++++++ .../flipcash/reporting/ReportDescription.kt | 27 ++++++++ .../com/flipcash/reporting/ReportReason.kt | 20 ++++++ .../reporting/ReportDescriptionTest.kt | 68 +++++++++++++++++++ .../internal/network/api/ReportingApi.kt | 6 +- settings.gradle.kts | 2 + 11 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 kmp/shared-core/spm/Sources/SharedCoreKit/ReportReason.swift create mode 100644 kmp/shared-core/spm/Tests/SharedCoreKitTests/ReportReasonTests.swift create mode 100644 libs/reporting/.gitignore create mode 100644 libs/reporting/build.gradle.kts create mode 100644 libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportDescription.kt create mode 100644 libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportReason.kt create mode 100644 libs/reporting/src/commonTest/kotlin/com/flipcash/reporting/ReportDescriptionTest.kt diff --git a/.github/workflows/shared-core-tests.yml b/.github/workflows/shared-core-tests.yml index c6abaf1806..5db736050b 100644 --- a/.github/workflows/shared-core-tests.yml +++ b/.github/workflows/shared-core-tests.yml @@ -11,6 +11,7 @@ on: - 'libs/codes/kikcode/**' - 'libs/currency-math/**' - 'libs/encryption/**' + - 'libs/reporting/**' - 'gradle/libs.versions.toml' - '.github/workflows/shared-core-tests.yml' @@ -71,7 +72,8 @@ jobs: :libs:encryption:hmac:iosSimulatorArm64Test \ :libs:encryption:mnemonic:iosSimulatorArm64Test \ :libs:encryption:sha256:iosSimulatorArm64Test \ - :libs:encryption:sha512:iosSimulatorArm64Test + :libs:encryption:sha512:iosSimulatorArm64Test \ + :libs:reporting:iosSimulatorArm64Test # The facade suite has to run against the Kotlin in this checkout, not against the last # published release, or a facade written for an unreleased Kotlin change is untested until diff --git a/kmp/shared-core/build.gradle.kts b/kmp/shared-core/build.gradle.kts index 219ed71f6f..f115b5c75e 100644 --- a/kmp/shared-core/build.gradle.kts +++ b/kmp/shared-core/build.gradle.kts @@ -45,6 +45,7 @@ kotlin { export(project(":libs:encryption:ed25519")) export(project(":libs:encryption:mnemonic")) export(project(":libs:currency-math:discrete-curve")) + export(project(":libs:reporting")) } } @@ -59,6 +60,7 @@ kotlin { api(project(":libs:encryption:ed25519")) api(project(":libs:encryption:mnemonic")) api(project(":libs:currency-math:discrete-curve")) + api(project(":libs:reporting")) } } } diff --git a/kmp/shared-core/spm/Sources/SharedCoreKit/ReportReason.swift b/kmp/shared-core/spm/Sources/SharedCoreKit/ReportReason.swift new file mode 100644 index 0000000000..cd7a3bded3 --- /dev/null +++ b/kmp/shared-core/spm/Sources/SharedCoreKit/ReportReason.swift @@ -0,0 +1,29 @@ +import Foundation +import SharedCore + +/// The Kotlin vocabulary, under a local name. Deliberately a typealias and not a Swift enum: +/// the tokens are the wire format, and there must be exactly one place they are written down. +public typealias ReportReason = SharedCore.ReportReason + +/// Assembles the `description` field of a `ReportRequest`, in the shared Kotlin. +public enum ReportDescription { + + /// The cap the composer enforces as you type. + public static let maxDetailsLength = Int(SharedCore.ReportDescription.shared.MAX_DETAILS_LENGTH) + + /// The `description` field of a `ReportRequest`: reason token, then the person's words. + public static func build(reason: ReportReason, details: String?) -> String { + SharedCore.ReportDescription.shared.build(reason: reason, details: details) + } +} + +public extension ReportReason { + /// The order the reasons are offered in. This *is* app UI, which is why it lives on the + /// Swift side -- but it must stay exhaustive, and `ReportReasonTests` checks that it is. + /// + /// The case names are the ones Kotlin/Native generates, which lowercases the whole word + /// rather than preserving the Kotlin hump: `scamorfraud`, not `scamOrFraud`. + static var displayOrder: [ReportReason] { + [.spam, .scamorfraud, .harassment, .sexualcontent, .violence, .other] + } +} diff --git a/kmp/shared-core/spm/Tests/SharedCoreKitTests/ReportReasonTests.swift b/kmp/shared-core/spm/Tests/SharedCoreKitTests/ReportReasonTests.swift new file mode 100644 index 0000000000..b44d1c6203 --- /dev/null +++ b/kmp/shared-core/spm/Tests/SharedCoreKitTests/ReportReasonTests.swift @@ -0,0 +1,29 @@ +import Foundation +import Testing +@testable import SharedCoreKit + +@Suite struct ReportReasonTests { + + @Test func buildsTheBareTokenWhenThereAreNoDetails() { + #expect(ReportDescription.build(reason: .spam, details: nil) == "spam") + } + + @Test func putsTrimmedDetailsOnTheSecondLine() { + #expect( + ReportDescription.build(reason: .scamorfraud, details: " details ") + == "scam_or_fraud\ndetails" + ) + } + + /// The cap crosses the bridge as an `Int32`; this catches a conversion that mangles it. + @Test func carriesTheDetailsCapAcrossTheBridge() { + #expect(ReportDescription.maxDetailsLength == 1000) + } + + /// The one that earns its keep: a seventh reason added in Kotlin compiles fine on both + /// sides and would be silently missing from the iOS sheet. + @Test func displayOrderCoversEveryReason() { + #expect(ReportReason.displayOrder.count == ReportReason.entries.count) + #expect(Set(ReportReason.displayOrder.map(\.token)).count == ReportReason.displayOrder.count) + } +} diff --git a/libs/reporting/.gitignore b/libs/reporting/.gitignore new file mode 100644 index 0000000000..42afabfd2a --- /dev/null +++ b/libs/reporting/.gitignore @@ -0,0 +1 @@ +/build \ No newline at end of file diff --git a/libs/reporting/build.gradle.kts b/libs/reporting/build.gradle.kts new file mode 100644 index 0000000000..a03de2ded7 --- /dev/null +++ b/libs/reporting/build.gradle.kts @@ -0,0 +1,34 @@ +plugins { + kotlin("multiplatform") + id("com.android.kotlin.multiplatform.library") +} + +kotlin { + android { + namespace = "com.flipcash.reporting" + compileSdk { + version = release(libs.versions.android.compileSdk.get().toInt()) { + minorApiLevel = libs.versions.android.compileSdkMinor.get().toInt() + } + } + minSdk = 29 + withHostTest {} + } + + iosArm64() + iosSimulatorArm64() + iosX64() + macosArm64() + macosX64() + + sourceSets { + commonMain { + // Pure Kotlin -- no external dependencies needed. + } + commonTest { + dependencies { + implementation(kotlin("test")) + } + } + } +} diff --git a/libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportDescription.kt b/libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportDescription.kt new file mode 100644 index 0000000000..74f42844b1 --- /dev/null +++ b/libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportDescription.kt @@ -0,0 +1,27 @@ +package com.flipcash.reporting + +object ReportDescription { + + /** + * The contract's cap is 8192 (protovalidate `max_len`, counted in characters). This is the + * cap the apps actually enforce as you type, chosen so nothing can approach the contract's. + */ + const val MAX_DETAILS_LENGTH: Int = 1000 + + /** + * Assembles the `description` field of a `ReportRequest`: the reason's token on the first + * line, and the person's own words, if any, from the second line on. + * + * Details are trimmed, and dropped entirely when they are blank -- an empty second line + * carries nothing, and a reader splitting on the first newline should not have to decide + * whether a trailing separator was meaningful. Details longer than [MAX_DETAILS_LENGTH] + * are truncated rather than rejected: this runs after the UI has already capped the field, + * so reaching it means a caller got past that, and losing the tail of an over-long report + * is better than losing the report. + */ + fun build(reason: ReportReason, details: String?): String { + val trimmed = details?.trim().orEmpty() + if (trimmed.isEmpty()) return reason.token + return reason.token + "\n" + trimmed.take(MAX_DETAILS_LENGTH) + } +} diff --git a/libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportReason.kt b/libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportReason.kt new file mode 100644 index 0000000000..f77ed7728d --- /dev/null +++ b/libs/reporting/src/commonMain/kotlin/com/flipcash/reporting/ReportReason.kt @@ -0,0 +1,20 @@ +package com.flipcash.reporting + +/** + * The reason a person gave for a report, and the token that carries it to the server. + * + * The contract (`flipcash2-client-protocol` 0.10.0) has no category field -- `ReportRequest` + * carries only a free-form `description`. These tokens are how a category survives that, + * so they are part of the wire format: renaming one changes what moderation receives, and + * `ReportDescriptionTest` exists to make that change fail loudly rather than silently. + * + * Labels, ordering and icons are deliberately not here. Those are per-app UI. + */ +enum class ReportReason(val token: String) { + Spam("spam"), + ScamOrFraud("scam_or_fraud"), + Harassment("harassment"), + SexualContent("sexual_content"), + Violence("violence"), + Other("other"), +} diff --git a/libs/reporting/src/commonTest/kotlin/com/flipcash/reporting/ReportDescriptionTest.kt b/libs/reporting/src/commonTest/kotlin/com/flipcash/reporting/ReportDescriptionTest.kt new file mode 100644 index 0000000000..e3a23ea0ed --- /dev/null +++ b/libs/reporting/src/commonTest/kotlin/com/flipcash/reporting/ReportDescriptionTest.kt @@ -0,0 +1,68 @@ +package com.flipcash.reporting + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class ReportDescriptionTest { + + @Test + fun `every reason has the token the server expects`() { + assertEquals("spam", ReportReason.Spam.token) + assertEquals("scam_or_fraud", ReportReason.ScamOrFraud.token) + assertEquals("harassment", ReportReason.Harassment.token) + assertEquals("sexual_content", ReportReason.SexualContent.token) + assertEquals("violence", ReportReason.Violence.token) + assertEquals("other", ReportReason.Other.token) + } + + @Test + fun `tokens are unique`() { + val tokens = ReportReason.entries.map { it.token } + assertEquals(tokens.size, tokens.toSet().size) + } + + @Test + fun `a reason with no details is the bare token`() { + assertEquals("spam", ReportDescription.build(ReportReason.Spam, null)) + assertEquals("spam", ReportDescription.build(ReportReason.Spam, "")) + assertEquals("spam", ReportDescription.build(ReportReason.Spam, " ")) + assertEquals("spam", ReportDescription.build(ReportReason.Spam, "\n\t ")) + } + + @Test + fun `details follow the token on the next line`() { + assertEquals( + "scam_or_fraud\nHe asked me to send cash to unlock a withdrawal", + ReportDescription.build( + ReportReason.ScamOrFraud, + "He asked me to send cash to unlock a withdrawal", + ), + ) + } + + @Test + fun `details are trimmed but internal newlines are kept`() { + assertEquals( + "other\nfirst\nsecond", + ReportDescription.build(ReportReason.Other, " first\nsecond "), + ) + } + + @Test + fun `details are truncated at the cap`() { + val long = "x".repeat(ReportDescription.MAX_DETAILS_LENGTH + 50) + val built = ReportDescription.build(ReportReason.Other, long) + assertEquals("other\n" + "x".repeat(ReportDescription.MAX_DETAILS_LENGTH), built) + assertTrue(built.length < 8192, "must stay well under the contract's max_len") + } + + @Test + fun `the first line is always a known token`() { + val tokens = ReportReason.entries.map { it.token }.toSet() + for (reason in ReportReason.entries) { + val built = ReportDescription.build(reason, "some details") + assertTrue(built.substringBefore('\n') in tokens) + } + } +} diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ReportingApi.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ReportingApi.kt index 64877e2a00..1e000b00c2 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ReportingApi.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/internal/network/api/ReportingApi.kt @@ -35,7 +35,6 @@ internal class ReportingApi @Inject constructor( ): RpcReportingService.ReportResponse { val builder = RpcReportingService.ReportRequest.newBuilder() .setDescription(description) - .apply { setAuth(authenticate(owner)) } when (target) { is ReportTarget.User -> builder.setUserId(target.userId.asUserId()) @@ -51,6 +50,11 @@ internal class ReportingApi @Inject constructor( ) } + // Auth last, always: `authenticate()` signs `buildPartial()`, so anything set after this + // line is outside the signature. The target used to be set below it, which meant every + // report was signed over its description alone. + builder.apply { setAuth(authenticate(owner)) } + val request = builder.build() request.validate().orThrow() diff --git a/settings.gradle.kts b/settings.gradle.kts index 71ee3ac63e..ff1f3436f6 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -239,6 +239,7 @@ include( ":libs:permissions:public", ":libs:quickresponse", + ":libs:reporting", ":libs:search", ":libs:vibrator:bindings", @@ -326,6 +327,7 @@ val kmpUnitTestModules = setOf( // (macOS .dylib / Linux .so); it can't load on the Linux CI runner. Its ed25519.json // parity is gated via the iOS cinterop path (macOS) instead — so it's excluded here. ":libs:encryption:utils", + ":libs:reporting", ) // ed25519 and mnemonic excluded: both pull in the JNI-backed Ed25519Kmp Android actual for their // host vector tests, which can't load on the Linux CI runner (see kmpUnitTestModules).