Skip to content

fix: harden rac ingest and outbox handling#1040

Open
charmful0x wants to merge 1 commit into
feat/rac@1.0from
impr/rac@1.0
Open

fix: harden rac ingest and outbox handling#1040
charmful0x wants to merge 1 commit into
feat/rac@1.0from
impr/rac@1.0

Conversation

@charmful0x

Copy link
Copy Markdown
  • require from-process for tagged RAC inbound -- removed signer fallback (so only process-to-process)
  • reject malformed/negative rac-slot/rac-ratchet
  • validate basic key shapes: non-empty binary recipient/channel/from-process
  • scrub toplevel outbound control fields before stamping
  • append outbox after highest canonical numeric key

end;
ratchet_rule(_) -> strict.
parse_ratchet(false) -> {ok, strict};
parse_ratchet(<<"false">>) -> {ok, strict};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just parse_ratchet(Atom) when is_atom(Atom) -> parse_ratchet(...) here if we are editing it anyway? Nasty clankerism made its way in.

%% @doc Admit or reject one inbound message. Untagged traffic (no `rac-slot')
%% passes through unchanged.
ingest(Base, In, Opts) ->
ingest(Base, In, Opts) when is_map(In) ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_map or ?IS_LINK I think? Ideally 'link to message', too. On at least one branch/edge we have an IS_MESSAGE that covers this. Not sure if it has landed yet though.

parse_slot(Value) ->
parse_nonneg_int(Value).

parse_nonneg_int(N) when is_integer(N), N >= 0 ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must have something for this already? It is the same logic as the scheduler and processes both need. Perhaps we should put it in lib_process_utils or similar?

{ok,
hb_maps:fold(
fun(Key, Value, Acc) ->
NormKey = hb_util:to_lower(hb_ao:normalize_key(Key)),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is something in hb_utils for this already. lower_case_map or similar.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we would just need a hb_maps:with rather than the fold.

2. Stamp the outbound message with `rac-slot = Slot` and `target = recipient`,
plus `rac-channel`/`rac-ratchet` when they differ from their defaults.
3. Append the stamped message to the outbox (`results/outbox`).
Existing top-level RAC control keys, `target`, and `from-*` provenance keys

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that the responsibility of ~push@1.0, not ~rac@1.0? I can kind of see the logic for it being here, but if I am seeing it correctly it would blow the edges of the abstraction. For example, if push@2.0 wanted to utilize rac@1.0 (or some other device construct), we would end up breaking it in a super counter-intuitive way that would require the dev/us to hack around it in another ugly way, etc.

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.

2 participants