Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions ACCEPTANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,21 @@ existing codebase, not to the change under review.
without comment syntax (JSON fixtures under `tests/`, `*.lock` files, binary
assets) cannot carry the notice; there is no committed allow-list or CI check
asserting headers. The B4 commands above are the interim verification.
- **A1b — two benign textual matches in the broker grep.** The repo-wide A1b
command matches `.claude/hooks/guard-edits.py` (the anti-broker edit guard's
own pattern list) and the "no broker" documentation comment in
`web/lib/potato_mesh/application/pubsub.rb` (PS1). Both are descriptive or
defensive text *about* the apex ban — neither is a broker dependency or
connection — but they sit outside A1b's `via_mqtt` exemption wording. Treat
these two files as documented exemptions until the A1b filter codifies them.
- **B1 — sandbox DNS breaks the `POST /api/instances` spec block.** In
sandboxed environments whose resolver maps the suite's test domains
(`mesh.example`, …) into the SSRF guard's restricted address ranges, 17
`spec/app_spec.rb` "POST /api/instances" examples fail with
`{"error":"restricted domain"}` (400 instead of 201). Environmental only:
the failures reproduce identically with and without any change under review
and do not occur where the test domains resolve normally (CI). Attribute to
the environment, not the codebase or the change.

---

Expand Down Expand Up @@ -2521,3 +2536,52 @@ federation wire, and Flutter app are behaviorally untouched by this batch —
the only edits outside the four fixes are the lockstep 0.7.2 version-bump
stamps (manifests, lockfiles, iOS plist, README pinned tags, S-A1), verified
by `tests/test_version_sync.py`.

---

## Bugfix: MeshCore ghost nodes (stale contact enrichment discarded)

A MeshCore node first seen via a bare `ADVERTISEMENT` push was upserted as a
minimal placeholder stamped `lastHeard = now` (receiver wall clock). The
follow-up roster contact record — carrying the real name/role/public key — is
stamped `lastHeard = last_advert`, the **sender-side** advert-creation time,
which is always older than the placeholder's receive time (seconds for healthy
clocks, years for broken ones). `upsert_node`'s row-level freshness guard
(`WHERE excluded.last_heard >= nodes.last_heard`) therefore discarded the
entire named update, permanently: every later contact re-post (auto-update,
periodic snapshot, restart) is also sender-stamped and also lost, while each
advertised-position ingest re-bumps the row's `last_heard`. Result: nameless
"ghost" nodes with a hex `short_name`, NULL role (displayed as the CLIENT
default), and an advert-stamped `position_time` — violating the reconciliation
promise in `CONTRACTS.md` ("a later full contact advertisement reconciles it",
SPEC A4e). Fixed web-side (Ruby): after the guarded upsert, a non-synthetic
record additionally **fills identity columns that are still NULL** (`num`,
`short_name`, `long_name`, `macaddr`, `hw_model`, `role`, `public_key`,
`is_unmessagable`) regardless of staleness — stale data can fill gaps but can
never overwrite fresher values, and synthetic placeholders remain barred from
real rows. No ingestor/API/DB-schema change; protocol-neutral (Invariant IV).

### GH-A1 — Stale contact records name advert-placeholder ghosts
```bash
( cd web && bundle exec rspec spec/data_processing_spec.rb -e "stale contact record enrichment" )
```
**Expected:** pass. Replaying the ingestor's wire sequence — bare-advert
placeholder (`lastHeard = now`, no name) followed by the roster contact record
(`lastHeard = last_advert`, older by 17 s and by ~2 years in a second example) —
leaves the node **named** with its real role and public key. The stale record
never regresses `last_heard`, never overwrites an existing name/role, empty
strings never fill `long_name` / `short_name` (the other identity fields of the
same record still fill), and a stale `synthetic=1` chat placeholder still
cannot touch a real row.

### GH-R1 — Regression: prior acceptance still holds
```bash
( cd web && bundle exec rspec ) && ( cd web && npm test ) && ( cd web && bundle exec rufo --check . )
( . .venv/bin/activate && pytest -q tests/ )
```
**Expected:** all green. At risk and explicitly required to stay green: the
pre-existing `upsert_node` guard specs (`data_processing_spec.rb` — role/
identity preservation, generic-name fallback, synthetic flag + merge #755/#803)
and `database_spec.rb`'s node-merge suites, since the fix appends a second
NULL-fill statement inside the same `upsert_node` transaction; the Python
ingestor is untouched (A4e's advert-capture suite unchanged).
2 changes: 1 addition & 1 deletion data/mesh_ingestor/CONTRACTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ The web application applies the same normalisation as a safety net so legacy ing

- *Contact roster (rich).* The startup `ensure_contacts()` fetch plus live `NEW_CONTACT` / `NEXT_CONTACT` pushes carry the full advert (name, role, position) and upsert complete node rows. This covers every node the radio has added to its contact book.
- *Auto-update re-fetch (freshness).* The provider sets `mc.auto_update_contacts = True`, so the meshcore library re-fetches **changed** contacts (incrementally, by `lastmod`) whenever an `ADVERTISEMENT` / `PATH_UPDATE` push arrives. A re-advert from a known node therefore refreshes its `last_advert` / position without waiting for a reconnect.
- *Bare advert (reach).* The `ADVERTISEMENT` (pubkey-only) push is also handled directly: for a public key **not** in the contact roster it upserts a minimal "heard now" node (`lastHeard`, `protocol`, `user.shortName`/`publicKey` only — no name/type/position), so radios running with auto-add off still register the advertiser. Known keys are skipped (the auto-update path keeps them fresh). The Ruby web app preserves an existing long name on conflict, so this placeholder never clobbers a richer record, and a later full contact advertisement reconciles it.
- *Bare advert (reach).* The `ADVERTISEMENT` (pubkey-only) push is also handled directly: for a public key **not** in the contact roster it upserts a minimal "heard now" node (`lastHeard`, `protocol`, `user.shortName`/`publicKey` only — no name/type/position), so radios running with auto-add off still register the advertiser. Known keys are skipped (the auto-update path keeps them fresh). The Ruby web app preserves an existing long name on conflict, so this placeholder never clobbers a richer record, and a later full contact advertisement reconciles it. Reconciliation does not depend on timestamp ordering: the contact record carries `lastHeard = last_advert` (the **sender-stamped** advert-creation time), which is always older than the placeholder's wall-clock stamp — the web app's node upsert therefore fills identity fields (name, role, public key, …) that are still NULL even from an older-stamped record, while timestamps/telemetry stay freshness-guarded (ACCEPTANCE GH-A1).

New protocols SHOULD likewise treat "node was heard" as a first-class, name-optional upsert so peer discovery does not hinge on a roster being populated.

Expand Down
71 changes: 59 additions & 12 deletions web/lib/potato_mesh/application/data_processing/node_writes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,6 @@ def touch_node_last_seen(
updated
end

# Insert or update a node row from an inbound NodeInfo-style payload.
#
# @param db [SQLite3::Database] open database handle.
# @param node_id [String] canonical node identifier.
# @param n [Hash] node payload extracted from the ingestor.
# @param protocol [String] protocol identifier (default +meshtastic+).
# @return [void]
# Read +hash[primary]+, falling back to the first present alias key. Lets the
# node ingest contract accept snake_case fields in addition to the Meshtastic
# camelCase the collector emits today; nil-aware so a boolean +false+ from the
Expand All @@ -171,6 +164,26 @@ def pick_alias(hash, primary, *aliases)
nil
end

# Insert or update a node row from an inbound NodeInfo-style payload.
#
# Two-phase write. Phase one is the freshness-guarded upsert: a record
# whose +lastHeard+ is older than the stored row cannot change
# timestamps, telemetry, or position. Phase two fills identity columns
# (+num+, +short_name+, +long_name+, +macaddr+, +hw_model+, +role+,
# +public_key+, +is_unmessagable+) that are still NULL, regardless of the
# record's staleness — a stale-but-richer record (e.g. a MeshCore roster
# contact stamped with the sender-side +last_advert+, which is always
# older than the wall-clock +lastHeard+ of the bare-advert placeholder
# that created the row) still names the node instead of being discarded
# wholesale, which produced permanently unnamed "ghost" nodes
# (ACCEPTANCE GH-A1). Synthetic chat placeholders never touch real rows
# in either phase.
#
# @param db [SQLite3::Database] open database handle.
# @param node_id [String] canonical node identifier.
# @param n [Hash] node payload extracted from the ingestor.
# @param protocol [String] protocol identifier (default +meshtastic+).
# @return [void]
def upsert_node(db, node_id, n, protocol: "meshtastic")
user = n["user"] || {}
met = pick_alias(n, "deviceMetrics", "device_metrics") || {}
Expand Down Expand Up @@ -219,6 +232,11 @@ def upsert_node(db, node_id, n, protocol: "meshtastic")
# sender names before the real contact advertisement is received.
synthetic = user["synthetic"] ? 1 : 0
long_name = pick_alias(user, "longName", "long_name")
short_name = pick_alias(user, "shortName", "short_name")
macaddr = user["macaddr"]
hw_model = pick_alias(user, "hwModel", "hw_model") || pick_alias(n, "hwModel", "hw_model")
public_key = pick_alias(user, "publicKey", "public_key")
is_unmessagable = coerce_bool(pick_alias(user, "isUnmessagable", "is_unmessagable"))

# If the incoming long name is a generic placeholder, prefer any real
# name already on record so we never stomp known data with fallback
Expand All @@ -240,13 +258,13 @@ def upsert_node(db, node_id, n, protocol: "meshtastic")
row = [
node_id,
node_num,
pick_alias(user, "shortName", "short_name"),
short_name,
long_name,
user["macaddr"],
pick_alias(user, "hwModel", "hw_model") || pick_alias(n, "hwModel", "hw_model"),
macaddr,
hw_model,
role,
pick_alias(user, "publicKey", "public_key"),
coerce_bool(pick_alias(user, "isUnmessagable", "is_unmessagable")),
public_key,
is_unmessagable,
coerce_bool(pick_alias(n, "isFavorite", "is_favorite")),
pick_alias(n, "hopsAway", "hops_away"),
n["snr"],
Expand Down Expand Up @@ -305,6 +323,35 @@ def upsert_node(db, node_id, n, protocol: "meshtastic")
AND NOT (COALESCE(nodes.synthetic,0) = 0 AND excluded.synthetic = 1)
SQL

# Ghost-node repair (GH-A1): the guard above skips records whose
# last_heard is older than the stored row — correct for timestamps,
# telemetry, and position, but it also starved identity data. A
# MeshCore roster contact is stamped with the sender-side
# last_advert, which is always older than the wall-clock lastHeard
# of the bare-advert placeholder that created the row, so the
# name/role/public key never landed and the node stayed a
# permanently unnamed ghost. Fill identity columns that are still
# NULL from any non-synthetic record regardless of staleness: gaps
# get filled, fresher values are never regressed, and synthetic
# chat placeholders remain barred from real rows. NULLIF keeps
# empty strings — a MeshCore contact may carry an empty adv_name,
# and shortName is guarded the same way — from filling
# long_name/short_name with blank text.
if synthetic.zero?
db.execute(<<~SQL, [node_num, short_name, long_name, macaddr, hw_model, role, public_key, is_unmessagable, node_id])
UPDATE nodes SET
num=COALESCE(num, ?),
short_name=COALESCE(short_name, NULLIF(?, '')),
long_name=COALESCE(long_name, NULLIF(?, '')),
macaddr=COALESCE(macaddr, ?),
hw_model=COALESCE(hw_model, ?),
role=COALESCE(role, ?),
public_key=COALESCE(public_key, ?),
is_unmessagable=COALESCE(is_unmessagable, ?)
WHERE node_id = ?
SQL
end

# Reconcile synthetic placeholder rows with their real counterparts
# whenever a MeshCore node is upserted. Both directions must fire —
# the arrival order of chat messages vs contact advertisements is
Expand Down
123 changes: 123 additions & 0 deletions web/spec/data_processing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,129 @@ def read_position(db, id)
end
end

# ---------------------------------------------------------------------------
# upsert_node — MeshCore ghost nodes: a bare-advert placeholder row (stamped
# with the receiver's wall clock) must not starve the follow-up roster
# contact record, whose lastHeard is the sender-stamped last_advert and is
# therefore always older (seconds for healthy clocks, years for broken
# ones). The freshness guard may skip the stale record's timestamps, but
# its identity fields (name, role, public key, …) must still fill columns
# that are NULL, or the node stays a permanently nameless "ghost".
# ---------------------------------------------------------------------------
describe "#upsert_node — stale contact record enrichment (ghost nodes)" do
include_context "with isolated db"

let(:pub_key) { "60e53d9b#{"ab" * 28}" }

# Mirror the ingestor's bare-advert placeholder (_advert_to_node_dict +
# radio metadata): wall-clock lastHeard, shortName/publicKey, no name/role.
def upsert_advert_placeholder(db, heard_at)
dp.upsert_node(db, "!60e53d9b", {
"lastHeard" => heard_at,
"lora_freq" => 869,
"modem_preset" => "SF8/BW62/CR8",
"user" => { "shortName" => "60e5", "publicKey" => pub_key },
}, protocol: "meshcore")
end

# Mirror the follow-up roster contact (_contact_to_node_dict): the full
# record named from the advert, stamped with the sender-side last_advert.
def upsert_contact_record(db, last_advert, long_name: "Alpha Repeater")
dp.upsert_node(db, "!60e53d9b", {
"lastHeard" => last_advert,
"lora_freq" => 869,
"modem_preset" => "SF8/BW62/CR8",
"user" => {
"longName" => long_name,
"shortName" => "60e5",
"publicKey" => pub_key,
"role" => "REPEATER",
},
}, protocol: "meshcore")
end

def node_row(db)
db.execute("SELECT * FROM nodes WHERE node_id = '!60e53d9b'").first
end

it "fills name and role from a contact record stamped seconds older (healthy clock)" do
db = open_db
upsert_advert_placeholder(db, now)
upsert_contact_record(db, now - 17)
row = node_row(db)
db.close
expect(row["long_name"]).to eq("Alpha Repeater")
expect(row["role"]).to eq("REPEATER")
expect(row["public_key"]).to eq(pub_key)
end

it "fills name and role from a contact record stamped years older (broken node clock)" do
db = open_db
upsert_advert_placeholder(db, now)
upsert_contact_record(db, now - 66_988_974)
row = node_row(db)
db.close
expect(row["long_name"]).to eq("Alpha Repeater")
expect(row["role"]).to eq("REPEATER")
end

it "keeps the newer last_heard when a stale contact record enriches the row" do
db = open_db
upsert_advert_placeholder(db, now)
upsert_contact_record(db, now - 17)
row = node_row(db)
db.close
expect(row["last_heard"]).to eq(now)
end

it "does not overwrite an existing name or role with stale data" do
db = open_db
upsert_contact_record(db, now, long_name: "Fresh Name")
dp.upsert_node(db, "!60e53d9b", {
"lastHeard" => now - 3600,
"user" => { "longName" => "Old Name", "shortName" => "OLD", "role" => "COMPANION" },
}, protocol: "meshcore")
row = node_row(db)
db.close
expect(row["long_name"]).to eq("Fresh Name")
expect(row["role"]).to eq("REPEATER")
end

it "does not fill a missing name with a stale empty adv_name" do
db = open_db
upsert_advert_placeholder(db, now)
upsert_contact_record(db, now - 17, long_name: "")
row = node_row(db)
db.close
expect(row["long_name"]).to be_nil
end

it "does not fill a missing short_name with a stale empty shortName" do
db = open_db
dp.upsert_node(db, "!60e53d9b", { "lastHeard" => now }, protocol: "meshcore")
dp.upsert_node(db, "!60e53d9b", {
"lastHeard" => now - 17,
"user" => { "shortName" => "", "longName" => "Alpha Repeater" },
}, protocol: "meshcore")
row = node_row(db)
db.close
expect(row["short_name"]).to be_nil
expect(row["long_name"]).to eq("Alpha Repeater")
end

it "still ignores stale synthetic placeholders for real rows" do
db = open_db
upsert_advert_placeholder(db, now)
dp.upsert_node(db, "!60e53d9b", {
"lastHeard" => now - 17,
"user" => { "longName" => "Chat Alias", "synthetic" => true },
}, protocol: "meshcore")
row = node_row(db)
db.close
expect(row["long_name"]).to be_nil
end
end

# ---------------------------------------------------------------------------
# upsert_node — synthetic flag + merge
# ---------------------------------------------------------------------------
Expand Down
Loading