Queued DM overwrite, storage bucket ownership by athallajovian9-cyber · Pull Request #142 · edge-toolkit/core

athallajovian9-cyber · 2026-10-12T13:49:56Z

Three findings from a review of the repository, each fixed with its regression test.

1. Queued direct messages overwrite each other (`libs/edge-toolkit/src/ws_server.rs`)

queue_direct stored pending messages keyed by sender id, so a second message queued from the same sender replaced the first: an offline recipient ended up with only the latest message, and the replaced one could never be acknowledged.

The map is now keyed by message id (already a UUIDv7 from connection.rs, which also keeps arrival order), so messages queue alongside each other. acknowledge_message looks entries up by the message_id carried on the message instead of by map key, which also acknowledges entries persisted under the old sender-keyed shape — covered by a dedicated test.

Proof:

cargo test -p edge-toolkit --test registry   →  9 passed, 0 failed  (7 before, +2 new)
  · queued_direct_messages_from_one_sender_do_not_replace_each_other
  · entries_persisted_with_the_legacy_sender_key_still_acknowledge

2. Storage writes are not authenticated to the bucket owner (`services/storage/src/routes.rs`)

PUT /storage/{agent_id}/{filename} only checked that the agent id exists, and the listener binds 0.0.0.0 (services/ws-server/src/main.rs:136), so any reachable client could write to another registered agent's bucket by naming its id.

The write is now accepted only from the address the agent registered from (last_known_ip, recorded at WebSocket connect) or from loopback, where the server's own runners and modules write on an agent's behalf. Anything else — including an agent that never registered a usable address (persisted as unknown) — gets the new StorageError::RequesterNotBucketOwner (403). Existence of the id is no longer treated as ownership.

The bind address is deliberately unchanged: serving agents across interfaces (QR/hotspot deployments) is the product. Ownership now travels with the request instead.

Proof (the four pre-existing PUT tests gained an explicit peer matching the registered address):

cargo test -p et-storage-service            →  all targets pass (11/11 in --test put, +4 new)
  · rejects_foreign_peer_with_403                      (cross-agent write blocked, store untouched)
  · accepts_peer_matching_the_registered_address
  · rejects_unverifiable_agent_address_with_403
  · accepts_loopback_peer_for_runner_writes
cargo check -p et-ws-service                →  clean (downstream callers unaffected)
cargo fmt --check                           →  clean

3. `cargo-deny-check` fails on `unsafe-libyaml` (RUSTSEC-2024-0449)

The crate is listed in the workspace's unmaintained metadata (Cargo.toml) but was missing from the advisory ignore list in config/deny.toml, so the audit errors:

BEFORE: error[unmistakable]: error[unmaintained]: unsafe-libyaml is unmaintained
AFTER:  advisories ok, bans ok, licenses ok, sources ok

Added the entry with the written rationale the file requires: reached via serde_yaml (registry save/load and operator-supplied CLI YAML — local files, not attacker input), already allowlisted for the unmaintained scan, no patched version exists, cleared only by a workspace-wide move to a safe-Rust parser.

Proof: cargo deny --config config/deny.toml check → advisories ok, bans ok, licenses ok, sources ok (verified before/after by stashing the entry).

Not included

No src/ behaviour change outside the two fixes; no dependency bumps.
The unauthenticated GET/HEAD on storage and the absence of any WS-level authentication are larger design questions than this PR — happy to open an issue to discuss them rather than smuggle an auth model in here.

…ne sender does not replace the first

…owns the bucket

…nale

coderabbitai · 2026-10-12T13:50:07Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: d7a005cc-6ff4-41da-b220-ead324c38664

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

deepsource-io · 2026-10-12T13:50:20Z

DeepSource Code Review

We reviewed changes in 06d728d...e690a58 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Updated (UTC)	Details
C#	Oct 12, 2026 1:50p.m.	Review ↗
C & C++	Oct 12, 2026 1:50p.m.	Review ↗
Docker	Oct 12, 2026 1:50p.m.	Review ↗
Java	Oct 12, 2026 1:50p.m.	Review ↗
JavaScript	Oct 12, 2026 1:50p.m.	Review ↗
Python	Oct 12, 2026 1:50p.m.	Review ↗
Rust	Oct 12, 2026 1:50p.m.	Review ↗
Secrets	Oct 12, 2026 1:50p.m.	Review ↗
Code coverage	Oct 12, 2026 2:20p.m.	Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

codacy-production · 2026-10-12T13:56:18Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 4 complexity · 22 duplication

Metric Results

Complexity 4

Duplication 22

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

athallajovian9-cyber · 2026-10-13T00:22:10Z

i feel like a dead person.

athallajovian9-cyber · 2026-10-13T00:22:23Z

making pull requests just to be ignored.

jayvdb · 2026-10-15T04:20:55Z

Sorry, reviewing now. Then you can rebase without the deny.toml

jayvdb · 2026-10-15T04:36:20Z

-            .insert(from_agent_id.to_string(), pending.clone());
+            .insert(pending.message_id.clone(), pending.clone());


The intent is to have only one message pending for an agent to another. This is to ensure that the size of the pending messages is kept predicable so that the amount of memory can be constrained and known. It is the responsibility of the agents to use this wisely. I am sorry this hasnt been clearly documented.

If you want a all messages kept, I would accept a Config to allow different modes of message retention.

jayvdb · 2026-10-15T04:47:20Z

+/// and a module or runner writing on an agent's behalf does so from this machine. Loopback is therefore
+/// accepted as well. Anything else -- including an agent that never registered a usable address (recorded as
+/// "unknown") or a request with no peer at all -- is a stranger and is refused.
+fn requester_owns_bucket(registered_ip: Option<&str>, peer: Option<std::net::SocketAddr>) -> bool {


Avoid using Option in function signatures.

The put logic should also be checking the AgentRecord.state to ensure the agent is still connected.

And we need to dig a bit more into why last_known_ip is an Option at all. And how/why req.peer_addr() can return None.

Agent IP are now mandatory per #89

codecov · 2026-10-15T07:37:42Z

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/storage/src/routes.rs	84.61%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

athallajovian9-cyber added 3 commits October 12, 2026 20:48

Keyed queued direct messages by message id so a second message from o…

61cd483

…ne sender does not replace the first

Refused storage bucket writes from peers that are not the agent that …

1f9d101

…owns the bucket

Ignored the unsafe-libyaml unmaintained advisory with a written ratio…

e690a58

…nale

jayvdb changed the title ~~Queued DM overwrite, storage bucket ownership, cargo-deny advisory gap~~ Queued DM overwrite, storage bucket ownership Oct 15, 2026

jayvdb reviewed Oct 15, 2026

View reviewed changes

Navigation Menu

Repository navigation

Queued DM overwrite, storage bucket ownership - #142

Queued DM overwrite, storage bucket ownership#142
athallajovian9-cyber wants to merge 3 commits into
edge-toolkit:mainfrom
athallajovian9-cyber:fix/queued-dm-storage-authz-deny-ignore

athallajovian9-cyber commented Oct 12, 2026

Uh oh!

coderabbitai Bot commented Oct 12, 2026

Uh oh!

deepsource-io Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

codacy-production Bot commented Oct 12, 2026

Uh oh!

athallajovian9-cyber commented Oct 13, 2026

Uh oh!

athallajovian9-cyber commented Oct 13, 2026

Uh oh!

jayvdb commented Oct 15, 2026

Uh oh!

jayvdb Oct 15, 2026

Uh oh!

jayvdb Oct 15, 2026

Uh oh!

jayvdb Oct 19, 2026

Uh oh!

codecov Bot commented Oct 15, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		.insert(from_agent_id.to_string(), pending.clone());
		.insert(pending.message_id.clone(), pending.clone());

Conversation

athallajovian9-cyber commented Oct 12, 2026

1. Queued direct messages overwrite each other (libs/edge-toolkit/src/ws_server.rs)

2. Storage writes are not authenticated to the bucket owner (services/storage/src/routes.rs)

3. cargo-deny-check fails on unsafe-libyaml (RUSTSEC-2024-0449)

Not included

Uh oh!

coderabbitai Bot commented Oct 12, 2026

Uh oh!

deepsource-io Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

codacy-production Bot commented Oct 12, 2026

Up to standards ✅

Uh oh!

athallajovian9-cyber commented Oct 13, 2026

Uh oh!

athallajovian9-cyber commented Oct 13, 2026

Uh oh!

jayvdb commented Oct 15, 2026

Uh oh!

jayvdb Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

jayvdb Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

jayvdb Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Oct 15, 2026

Codecov Report

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. Queued direct messages overwrite each other (`libs/edge-toolkit/src/ws_server.rs`)

2. Storage writes are not authenticated to the bucket owner (`services/storage/src/routes.rs`)

3. `cargo-deny-check` fails on `unsafe-libyaml` (RUSTSEC-2024-0449)

deepsource-io Bot commented Oct 12, 2026 •

edited

Loading