Skip to content

Disable reducer arg broadcast #2694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

suirad
Copy link

@suirad suirad commented May 2, 2025

Description of Changes

By default, reducer callbacks are broadcasted to all clients of the module. This PR adds a command line flag to spacetime start that when given, disables reducer arguments from being sent along with the reducer callback. It does so by swapping out the Args struct to the null version of it.

Potentially solves #1837 or at least it could temporarily.

API and ABI breaking changes

None; adds an optional command line flag to spacetime start to enable new behavior. --no-reducer-args

Expected complexity level and risk

Level is about 2. The change is pretty trivial, however the depth of that change to make it command line configurable adds a bit of complexity. There may be some unknowns that I am not aware of by not broadcasting the full args of reducers to other clients. This is mostly why I didn't make a more substantive change.

Testing

Successfully ran the repo test suite.

Additional tests against existing projects and a project to specifically to demonstrate the corrected behavior via extracting passwords from third party reducer args was done by @Lethalchip. See below images:

Setup

image
image

Use Case

image

Results

image
image

@suirad suirad force-pushed the censor-reducer-args branch from 83ef792 to 2cc6e17 Compare May 2, 2025 20:58
@CLAassistant
Copy link

CLAassistant commented May 2, 2025

CLA assistant check
All committers have signed the CLA.

@gefjon
Copy link
Contributor

gefjon commented May 5, 2025

Thank you so much for the contribution! We're aware of this issue and we actually have a planned fix to this issue that we're planning on going with. It has several benefits over this PR. Namely, the issues I see with this solution are:

  • This solution is only applicable to self-hosted SpacetimeDB-standalone, not to managed SpacetimeDB-cloud.
  • I perceive significant risk of developing a module with the intention of running it with argument broadcast disabled, but mistakenly deploying it with argument broadcast enabled.
  • This solution destroys all reducer argument information, and so any module which wants to send any sensitive information in reducer arguments at all is unable to inspect any other, non-sensitive arguments in reducer callbacks.
  • Many types do not have obvious empty/null/sentinel values, and our various client SDKs will almost certainly not correctly handle receiving ArgsTuple::nullary() instead of an args tuple which conforms to the reducer's argument type. For example, I strongly suspect that this will break the Rust client SDK.

What we actually want is a way to annotate reducers that their event broadcasting should be caller-only. This is a property of the specific reducer, not the database or the host process. When the SubscriptionManager goes to broadcast a transaction resulting from a caller-only reducer, it sends a TransactionUpdateLight message to all of the clients other than the caller, rather than a full TransactionUpdate. The client SDKs will recognize this and expose the event as Event::UnknownTransaction, and will not fire reducer callbacks for it.

This is likely to be too involved for a community contribution, just from the perspective of testing and review. It'll require modifications to our ModuleDefs, both the Rust and C# module bindings libraries including their macro syntaxes, and also testing with all three of our client SDKs to ensure they correctly handle TransactionUpdateLight messages.

@gefjon gefjon closed this May 5, 2025
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.

3 participants