Skip to content

feat(reactions): attribute relayed reactions to the reactor via originator-identity stamp#1006

Open
thatSFguy wants to merge 2 commits into
torlando-tech:mainfrom
thatSFguy:feat/reaction-originator-attribution
Open

feat(reactions): attribute relayed reactions to the reactor via originator-identity stamp#1006
thatSFguy wants to merge 2 commits into
torlando-tech:mainfrom
thatSFguy:feat/reaction-originator-attribution

Conversation

@thatSFguy

Copy link
Copy Markdown

Problem

When reactions are relayed through a re-originating group relay (e.g. reticulum-forwarding-service), Columba currently attributes all of them to the relay. The relay re-signs each reaction as itself, and a reaction has no body to carry authorship — so the carrying source_hash is the relay's, and ReactionWireCodec.parseCanonical sets sender = sourceHashHex accordingly.

Columba already speaks the canonical fields[0x40] = {0x00: target, 0x01: emoji} shape (emit + parse), so reactions through the relay already render — they're just all collapsed onto the relay's identity.

Fix

A re-originating relay stamps the reactor's source_hash (its lxmf.delivery destination hash — the same value a direct reaction carries, and what contacts are keyed by) into upstream's app-convention custom fields:

fields[0xFB] FIELD_CUSTOM_TYPE = "originator-identity"
fields[0xFC] FIELD_CUSTOM_DATA = <reactor source_hash, raw 16 bytes>

This PR teaches ReactionWireCodec.parseCanonical to use that stamp as sender when present, falling back to source_hash for direct (unstamped) reactions.

  • Purely additive / backward compatible — the 0x40 wire format is untouched; direct reactions carry no stamp.
  • Shared codec — the change is in rns-api's ReactionWireCodec, so both the kotlin-native and python backends pick it up.
  • No serializer change neededAppDataParser.serializeFieldsToJson already passes the top-level 251/252 fields through, and serializeFieldValue hex-encodes the 0xFC bytes.
  • It also aligns with how Columba already identifies own reactions (by identity), since the stamped source_hash is a stable per-reactor key that resolves against contacts (keyed by destination hash).

Spec basis

The value is the reactor's source_hash (destination hash), per the Reticulum/LXMF spec: §5.9.8 (reaction attribution rides on source_hash) and §9.1 (an LXMF source_hash is the destination hash; contacts are keyed by it — using the raw identity hash would orphan the lookup). The 0xFB/0xFC fields are the reserved app-convention fields (§5.9.1). Full convention, with rationale and the cross-client decode notes, is documented at reticulum-forwarding-service/docs/reaction-attribution.md.

Changes

  • LxmfFields.kt — add FIELD_CUSTOM_TYPE (0xFB) + FIELD_CUSTOM_DATA (0xFC).
  • ReactionWireCodec.kt — read the originator-identity stamp in parseCanonical; KDoc updated.
  • ReactionWireCodecTest.kt — stamp overrides source as sender; non-matching tag ignored; unstamped direct reactions unchanged. Tests serialize through the real AppDataParser to exercise the exact wire encoding.

Caveat

I don't have an Android/Gradle toolchain here, so I could not run ./gradlew :rns-api:test — please run it before merge. The change is small, logic-verified against serializeFieldValue's behaviour, and style-matched to the existing tests.

Context: this follows up #926.

…nator-identity stamp

Reactions relayed through a re-originating group relay (e.g.
reticulum-forwarding-service) are currently all attributed to the relay:
the relay re-signs each reaction as itself, and a reaction has no body to
carry authorship, so the carrying source_hash is the relay's.

Such a relay stamps the reactor's source_hash (its lxmf.delivery
destination hash — the same value a direct reaction carries and what
contacts are keyed by) into upstream's app-convention custom fields:
FIELD_CUSTOM_TYPE (0xFB) = "originator-identity", FIELD_CUSTOM_DATA
(0xFC) = reactor source_hash. This teaches ReactionWireCodec to use that
stamp as `sender` on the canonical 0x40 path when present, falling back
to source_hash for direct reactions.

Purely additive and backward compatible: the 0x40 wire format is
untouched, direct reactions carry no stamp, and the change lives in the
shared rns-api codec so both the kotlin-native and python backends pick
it up. Convention documented at
thatSFguy/reticulum-forwarding-service:docs/reaction-attribution.md.
@greptile-apps

greptile-apps Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds relay attribution support so that reactions forwarded by a re-originating group relay (e.g. reticulum-forwarding-service) are attributed to the actual reactor rather than the relay. The codec reads an optional FIELD_CUSTOM_TYPE (0xFB) = "originator-identity" / FIELD_CUSTOM_DATA (0xFC) = <reactor source_hash> stamp from the top-level field map and uses it as sender in the normalized output, falling back to source_hash for unstamped direct reactions.

  • LxmfFields.kt — adds FIELD_CUSTOM_TYPE = 0xFB and FIELD_CUSTOM_DATA = 0xFC constants with thorough KDoc matching the upstream LXMF spec.
  • ReactionWireCodec.kt — adds originatorIdentityHex() with hex-format validation (32-char, all hex digits after lowercasing), wires the result into parseCanonical, and includes a clear ⚠️ TRUST warning noting the stamp is unauthenticated and that the caller is responsible for the relay trust gate.
  • ReactionWireCodecTest.kt — adds five new tests covering the happy path, non-matching type tag, absent FIELD_CUSTOM_DATA, and four malformed-data cases (wrong length, non-hex characters).

Confidence Score: 4/5

The codec change is additive and backward-compatible; the validation and tests are solid, but the caller (MessagingViewModel.handleIncomingReaction) does not yet implement the relay-context trust gate that the KDoc explicitly requires.

The stamp parsing, hex-format validation, and five new round-trip tests are all correct. The codec itself is clean. The open gap is that handleIncomingReaction reads only the sender field (the potentially-overridden reactor identity) and never consults source_hash to verify the reaction actually arrived via a trusted relay — which is the check the KDoc says the caller must perform. Until that check is in place, any direct peer can forge reaction attribution to an arbitrary identity.

app/src/main/java/network/columba/app/viewmodel/MessagingViewModel.kt — handleIncomingReaction needs to verify source_hash against a trusted relay before accepting the stamp override.

Important Files Changed

Filename Overview
rns-api/src/main/java/network/columba/app/rns/api/util/LxmfFields.kt Adds FIELD_CUSTOM_TYPE (0xFB) and FIELD_CUSTOM_DATA (0xFC) constants adjacent to the pre-existing FIELD_CUSTOM_META (0xFD); well-documented with spec references and no conflicts with existing field IDs.
rns-api/src/main/java/network/columba/app/rns/api/util/ReactionWireCodec.kt originatorIdentityHex() correctly validates hex format and length before accepting the stamp; parseCanonical wires it in with source_hash preserved; KDoc documents the unauthenticated-stamp trust concern but defers the relay-context check to the caller (MessagingViewModel.handleIncomingReaction), which currently does not implement it.
rns-api/src/test/java/network/columba/app/rns/api/util/ReactionWireCodecTest.kt Five new tests cover stamp override, mismatched type tag, absent data, and four malformed-data cases; all serialize through the real AppDataParser to exercise the actual wire encoding boundary.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Reactor
    participant Relay as Re-originating Relay
    participant Columba as Columba Client

    Note over Reactor,Relay: Direct reaction (unstamped)
    Reactor->>Columba: "LXMessage{fields[0x40]={target, emoji}}, source_hash = Reactor"

    Note over Reactor,Relay: Relayed reaction (stamped)
    Reactor->>Relay: "LXMessage{fields[0x40]={target, emoji}}, source_hash = Reactor"
    Relay->>Columba: "LXMessage{fields[0x40]={target, emoji}, fields[0xFB]="originator-identity", fields[0xFC]=Reactor.source_hash}, source_hash = Relay"

    Note over Columba: ReactionWireCodec.parseCanonical()
    Columba->>Columba: originatorIdentityHex(fields)? Reactor.source_hash (stamp present, validated) else sourceHashHex (unstamped)
    Columba->>Columba: "normalized{sender=Reactor, source_hash=Relay}"
    Columba->>Columba: handleIncomingReaction() mergeReactionIntoJson(sender)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Reactor
    participant Relay as Re-originating Relay
    participant Columba as Columba Client

    Note over Reactor,Relay: Direct reaction (unstamped)
    Reactor->>Columba: "LXMessage{fields[0x40]={target, emoji}}, source_hash = Reactor"

    Note over Reactor,Relay: Relayed reaction (stamped)
    Reactor->>Relay: "LXMessage{fields[0x40]={target, emoji}}, source_hash = Reactor"
    Relay->>Columba: "LXMessage{fields[0x40]={target, emoji}, fields[0xFB]="originator-identity", fields[0xFC]=Reactor.source_hash}, source_hash = Relay"

    Note over Columba: ReactionWireCodec.parseCanonical()
    Columba->>Columba: originatorIdentityHex(fields)? Reactor.source_hash (stamp present, validated) else sourceHashHex (unstamped)
    Columba->>Columba: "normalized{sender=Reactor, source_hash=Relay}"
    Columba->>Columba: handleIncomingReaction() mergeReactionIntoJson(sender)
Loading

Reviews (2): Last reviewed commit: "review: validate stamp format, document ..." | Re-trigger Greptile

…dge cases

Addresses the Greptile review on the originator-identity stamp:

- Validate FIELD_CUSTOM_DATA is exactly a 32-hex-char destination hash
  before using it as `sender`; a blank/wrong-length/non-hex value is
  rejected so it can never become an unresolvable key in reactionsJson.
- Document the trust assumption prominently: the stamp is UNAUTHENTICATED
  and safe only when the reaction arrived via a trusted relay. Since the
  codec has no DB context, the final trust gate (honor the override only
  when source_hash matches the relay/group that delivered the reacted-to
  message) belongs in MessagingViewModel.handleIncomingReaction — flagged
  at both the helper and the call site. source_hash is preserved in the
  normalized output so the caller can make that decision.
- Add tests: stamp-type-present-but-data-absent → fallback; malformed
  (wrong length / non-hex) data → fallback.
thatSFguy added a commit to thatSFguy/reticulum-forwarding-service that referenced this pull request Jun 19, 2026
…for the stamp

The originator-identity stamp is unauthenticated, so a receiver that
honors it from any source lets a direct peer forge reactions attributed
to a third party. Document the two receiver MUSTs: validate 0xFC is a
well-formed 16-byte source_hash, and only honor the stamp when the
reaction arrived via a trusted relay (carrying source_hash matches the
relay/group that delivered the reacted-to message); else fall back to
source_hash. Surfaced by the Greptile review on torlando-tech/columba#1006.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thatSFguy

Copy link
Copy Markdown
Author

Thanks for the thorough review — the security finding (#1) is valid and important; I should have flagged the trust model in the original PR. Pushed 242452c addressing all three:

#2FIELD_CUSTOM_DATA validationoriginatorIdentityHex now requires exactly a 32-hex-char (16-byte) value (lowercased, all-hex), so a blank/wrong-length/non-hex stamp is rejected and can never land in reactionsJson as an unresolvable key.

#3 — edge-case tests ✅ Added: stamp type present but FIELD_CUSTOM_DATA absent → fallback; and malformed values (too short, too long, non-hex) → fallback. The malformed cases pass the value as a String (what an attacker's msgpack str produces) so they exercise the non-hex guard, not just length.

#1 — unauthenticated stamp / spoofing — agreed, this is the real one. The stamp is an unsigned assertion: its trust comes entirely from the relay (a re-originating relay verifies the reactor's LXMF signature before stamping, the same way you already trust the relay's authorship on relayed text). So the correct fix is a receiver-side trust gate: honor the override only when the reaction provably arrived via a trusted relay — concretely, when the carrying source_hash matches the relay/group source that delivered the reacted-to message; otherwise ignore it and attribute by source_hash.

That gate needs DB/conversation context (the target message's source), which lives in MessagingViewModel.handleIncomingReaction, not in this pure codec. So in this PR I've: (a) validated the value, (b) documented the trust assumption prominently at both the helper and the parseCanonical call site, and (c) preserved source_hash in the normalized output so the ViewModel can make the call. I deliberately did not implement the gate here because it depends on how Columba models a "trusted relay/group" — happy to follow up with that change if you point me at the right signal (a group/relay flag on the conversation, an allowlist, etc.).

I also made the trust gate a normative receiver requirement in the upstream convention doc so every client implements it consistently: https://github.com/thatSFguy/reticulum-forwarding-service/blob/main/docs/reaction-attribution.md#security

(Still couldn't run ./gradlew :rns-api:test here — no Android toolchain — so a CI/local run before merge is appreciated.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant