feat(rtbf): declare which data store keys hold a user's data - #35
Merged
Conversation
## The Configs API is one transport, not one repository
`/creator-configs-public-api/v1/.../repositories/{repository}` takes the
repository as a path parameter, and the vendored spec's `Repository` enum
has eight values. `rbx-config` had it as `const REPOSITORY:
"InExperienceConfig"`, which made the other seven unreachable and would
have made any second consumer copy the client.
That client is now `rbx_core::api::configs`, repository-agnostic, for the
reason `send_with_csrf` moved there: four crates carried their own copy
until they started disagreeing. Leaf tools depend on rbx-core alone in
this workspace, and only aggregators depend on peers, so a second tool
reaching the Configs API through `rbx-config` would have been the first
peer-to-peer edge.
`rbx config` gains `--repository`, with `rbxconfig.toml`'s new
`repository` field as the source of truth for the commands that read it.
A flag contradicting the file is refused naming both, rather than
resolved by picking a winner: a publish into the wrong repository
replaces a live config wholesale and cannot be undone.
Two bugs found while reading that code. `previousDraftHash` was threaded
through `overwrite_draft` and hardcoded to `None`, so a sync silently
discarded a draft somebody had staged in the Creator Hub; the draft is now
read first and its hash sent, which is Roblox's own concurrency check, and
what was replaced is named on the way past. And the documented limits, 100
keys per repository and 256 characters per key, were not checked locally,
so the 101st key failed as a 400 mid-publish; `--dry-run` now refuses it
too, since a dry run reporting a clean plan for a publish that cannot
happen is the wrong answer.
## rbx rtbf
`DataStoresConfig` holds right-to-be-forgotten deletion templates: which
data store keys hold a user's data, so Roblox can delete them when a
request arrives. `rbx config --repository DataStoresConfig` can push them
today, and that is exactly the problem, because its entry model holds an
opaque value and every mistake in this file is one Roblox *accepts* and
then silently never matches.
`{UserId}` is case-sensitive: `{userId}` is stored happily and deletes
nothing. A pattern with no token names one fixed key belonging to nobody.
A whole-store template only works on standard stores. An omitted scope
means `global`, which is wrong if your keys are scoped. None of those
produce an error from Roblox, and you find out when a legal request goes
unfulfilled inside thirty days.
So the templates are typed, in `rbxrtbf.toml`, with those rules checked
locally, and `rbx rtbf verify` answers the question `check` cannot: does
each template name a data store that exists. A pattern is matched by
requiring digits where the token is, so `Player_{UserId}_Save` does not
match `Player_Settings_Save`: a verify that says yes too easily is worse
than no verify. Ordered stores are reported as unchecked rather than
missing, because Open Cloud does not list them and a false alarm teaches
people to ignore the command.
No lockfile, for the reason `rbx config` has none: the published config is
readable in full, so the remote state is a fetch. The templates are not
per env either, because a key naming scheme belongs to a codebase rather
than an environment, so `--env all` publishes the same declaration to
each universe.
Wired the way every declarative tool here is: the binary, `rbx check`
(two rows, so the local rules still run under `--offline`), the JSON
schema, `rbx doctor`'s scope coverage, and the docs. `rbx import` does not
adopt templates yet; `docs/import.md` says so under what it cannot import,
and says to pull before writing that file by hand.
The crate shipped with forty unit tests and no integration test at all,
which left every path that talks to Roblox unexercised, including the one
that writes a compliance artefact. `wiremock` was a dev-dependency
nothing used.
Twelve tests now drive `run` through clap, against a mock server. The one
that matters asserts the payload from Roblox's own RTBF guide, written out
by hand rather than generated, at the `DataStoresConfig` path: a `sync`
that published under the wrong repository, or dropped the `{UserId}` token
on the way to the wire, would be a legal obligation quietly unmet and no
local test would have noticed.
They found a bug on the first run. `check` compared `published ==
declared`, which is `Vec` equality and therefore order-sensitive, while
`render` (which sorts, and exists precisely because declared order carries
no meaning) was only used for display. So a file somebody had tidied
reported as drift and was told to publish a no-op revision. The unit test
that was supposed to cover this asserted on `render` rather than on the
comparison, which is the same shape of gap as the tests it replaced.
Also pinned: `previousDraftHash` really travels back, a refused template
costs no request, `pull` refuses rather than truncating a template it
cannot model, an ordered store is unchecked rather than missing, and
`pull` and `verify` refuse a plural selector.
The repository states a contract, in its README and its 0.1.0 notes: `--json` on the reads writes one document to stdout and nothing else, with documented field names and a `schema_version`. Every declarative neighbour keeps it: `config get/list/versions`, `shop show`, `place versions`. `rbx rtbf` shipped without it, and `crates/rbx/tests/json_declared.rs` was left excluding rtbf rather than extended, which made the gap invisible instead of documented. `show` carries declared state: the file, the count, Roblox's ceiling, and one list of templates discriminated by `kind` rather than the file's two arrays, because a consumer filters and the two-array shape exists in the TOML for a human. Every entry carries the sample key Roblox will look for, which is the field a reviewer holds against the Luau. A defaulted scope reports as `global` rather than as an absence: `global` is what Roblox matches on, and the file's silence is not the answer. `verify` carries the answer nothing else produces, so a CI step reads `.ok` instead of grepping a listing for a red cross. Its `verdict` has three values rather than being a boolean, and that is the load-bearing detail: `unverifiable` is a limit of Open Cloud rather than a broken template, so it is excluded from `ok`, and a consumer folding it into a failure would break a build over an ordered store nothing can list. The document is emitted before the process exits 2, so a failing run still writes it. `check` gets none, matching every other per-tool check here: `rbx check --json` is the machine-readable drift document, and two shapes for one question is how a consumer comes to read the wrong one. Field tables in docs/rtbf.md, and the contract test now covers rtbf, including that a file the tool refuses leaves stdout empty.
…gaps
Three adversarial reviews of this branch. Every finding here was real; the
worst of them was mine and would have shipped.
## The concurrency guard sent a field name that does not exist
Roblox's configs guide says `previousDraftHash` in prose. The vendored
spec's `UpdateDraftRequest` defines `draftHash`, describes it as "the
previous draft hash for concurrency control", carries
`additionalProperties: false`, and does not contain the string
`previousDraftHash` anywhere. So the guide's spelling would have been
**rejected**, not ignored, and every `sync` against a repository with a
staged draft would have failed on an opaque 4xx: strictly worse than the
unguarded overwrite this change set out to fix.
The wiremock test could not catch it, because its body matcher asserted
the tool's own spelling. That is the same blind spot as the apikey
fixtures earlier on this branch family: a client checked only against
itself.
## rbx-config
`pull` stamped the file-wide `repository` field on the strength of a flag
that refreshed one env. The prompt says "leaves N other env(s)
untouched", and their entries were, but the repository they publish into
had moved: `pull --repository DataStoresConfig --env dev` followed by
`sync --env prod` would have replaced the universe's RTBF deletion
templates with prod's feature flags. It now refuses when the file
describes an env the pull did not touch.
The 404-versus-403 rule lost its tests when the client moved here.
Restored, plus three the old module did not have: the same distinction on
`get_draft`, the repository as the path segment, and an over-limit entry
set costing no request.
## rbx-rtbf
`sync` republished from the local file without ever reading the published
one, so a template this build cannot parse was silently gone, permanently,
on a legal artefact. `pull` refuses to lose one from a text file; sync did
it to the live set. It now reads first and refuses, and `check` no longer
claims sync leaves those alone.
`pull` never validated what it wrote, so it blessed a miscased token with
a green `Updated` and exit 0, and then every other command hard-errored on
the file pull had just written. It now writes (so there is somewhere to
fix it) and reports.
`validate` refused a documented working configuration: Roblox's rule is
that the id must be in the key name **or the scope**, so a constant key
under a per-user scope is legal and was rejected with a message claiming
Roblox would delete nothing, which was wrong on its own terms.
The near-miss scan stopped at the first correct token, so
`User_{UserId}_{userid}` passed. Roblox substitutes the first and leaves
the second as literal text: inert, which is the failure this crate exists
to catch. It now scans regardless, and pairs each closing brace with the
nearest opener, so a stray `{` cannot launder a miscased token.
The store walk truncated at 5000 stores and returned a short list as
though complete, manufacturing false `missing` findings: an accusation
that a compliance template is inert, produced by this function giving up.
It refuses instead.
`sync` resolved the API key after asking to publish.
## Unknown keys stay warnings
The first attempt closed `Templates`, `EntryConfig` and `EnvConfig` with
`deny_unknown_fields`. `rbx-schema`'s own test caught it: this repository
warns on unknown keys and never rejects them, so a key from a newer
release stays loadable and the generated schema is never stricter than the
tool. Reverted, and `rbxrtbf.toml` gained the warning instead, which is
what actually closes the hole: `[[keys]]` parses to an empty declaration
and `sync --yes` would publish that as a wipe, and the only previous
signal was a count in a prompt `--yes` skips.
## rbx-check
`Tool::Rtbf` moved ahead of `Tool::Config` to honour the documented
local-work-first ordering, and the test that pins it now asserts the rule
rather than a literal file-name sequence: it had been edited to accept the
wrong order, so it asserted a sequence its own name contradicted and would
have waved the next insertion through.
`docs/check.md` listed the emitted `tool` and `check` values as closed
sets and both were missing rtbf, so a pipeline filtering on the documented
contract silently dropped the rows reporting an unmet deletion obligation.
A new subcommand is not one page. Nine sites in this tree enumerate the command surface, and a tenth counts it, so shipping `rbx rtbf` without walking them leaves a site that contradicts its own binary. Found by auditing every file under `docs/` against the code rather than reading the new page: - `ARCHITECTURE.md`: the crate layout listed eleven crates, not twelve. - `docs/index.md`: `rtbf` was missing from the declarative pillar and from the "Every command" table, which is the page a newcomer reads first. - `docs/doctor.md`: the per-config scope table `rbx doctor` derives its "none of these are here" answer from named four files, and the code reads five. - `docs/check.md`: the `--json` list omitted `rtbf show/verify` and `secret list/public-key`. - `docs/teams.md`: "only `config/live` talks to Roblox" and the list of commands that compare against Roblox both predate `rtbf/live`. - `README.md`: the operational pillar still said `publish`, renamed to `message` two releases ago, and the editor `settings.json` block associated five schemas with six shipped. - `book.toml`: the sidebar comment counted twenty-six pages across five sections; the real numbers are twenty-eight and six. Two links also escaped the mdBook root, so they 404 on the published site while resolving fine on GitHub: `docs/open.md` to `THIRD-PARTY-NOTICES.md` and `docs/meta.md` to the README's editor-support anchor. Both now point at the repository over https, which works from either surface.
…ed templates Three adversarial reviews of the two fix commits on this branch, which nobody had reviewed. Ten findings, four of them P1, two of those silent unrecoverable loss of data held by Roblox. **A config publish cleared the repository's conditional rules.** The Configs API reads an omitted `conditionalRules` on `draft:overwrite` as an instruction: "when omitted on overwrite, all published conditional rules are cleared". This tool never sent the property, so the first `rbx config sync` against a repository carrying rules deleted all of them, with nothing in the replaced-draft report to say so, and a surviving entry referencing a deleted conditional then failed with an opaque 4xx. The rules are now restated from the draft when it stages any and from the published configuration when it does not, which is the layering the API states on the `PATCH` side of the same field. The second half is the half that matters: a first sync usually meets published rules and no draft, so echoing the draft alone would have left the loss where it was. They travel as opaque JSON, because on overwrite a property this code cannot name is a rule it would delete. **`rbx rtbf sync --yes` wiped every deletion template on a typo.** `[[keys]]` for `[[key]]` parses to an empty declaration, and each layer that looks like it should catch that deliberately does not: an empty file is a legitimate "delete nothing", `validate()` passes it, the read-before-write loop only bails on templates it cannot parse, and `--yes` skips the prompt. So a one-character typo published an empty set and cleared a universe's right-to-be-forgotten templates, exit 0, green in CI. `sync` and `verify` now refuse a declaration that is empty *while the file names a root table this release does not read*, which is the distinction that leaves the empty-file design intact. The doc comment that claimed the stderr warning already closed this is corrected: a warning does not change an exit status, and `--yes` reads no output. Both had the same root cause as the bug they were introduced beside: a client checked only against itself. So the wire names are now pinned against `spec/openapi.json` rather than against a mock this repository writes, `rbx config rollback` stops addressing a `:restore` custom method that appears nowhere under `creator-configs` in that document, and the spec-drift exemption that had quietly stopped scanning the whole configs surface is corrected, along with the stale `reqwest` dependency that was the only thing still making `rbx-config` look like an HTTP crate. The rest were unpinned guards and unfinished sweeps: the multi-env `pull` refusal had no test and could be deleted green, the unknown-root-key warning was asserted only by its own test's name, the `publish` to `message` rename survived in eight more places including two issue templates, and `rbx rtbf` was missing from a tenth enumeration of the command surface plus the bug template's config list.
…ry-and-rtbf # Conflicts: # CHANGELOG.md
`cargo doc --workspace` fails the build on the CI Doc job because three public items point at `OverwriteBody` and `ConfigsClient::conditional_rules_to_restate` through intra-doc links, and both targets are private. A reader of the public docs could not follow those links anyway. The references are worth keeping, so they become plain code spans rather than links. Nothing about what the docs say changes.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Started from the observation that
rbx confighardcodedInExperienceConfigwhere the API takes a path parameter.The Configs API is one transport, not one repository
/creator-configs-public-api/v1/configs/universes/{id}/repositories/{repository}takes the repository as a path parameter, and the vendored spec'sRepositoryenum has eight values:InExperienceConfig,RecommendationServicesConfig,DataStoresConfig,ExtendedServicesConfig,LeaderboardsConfig,ExperienceUserConfig,JourneysConfig,AntiCheatConfigcrates/rbx-config/src/api/mod.rs:13had it asconst REPOSITORY: &str = "InExperienceConfig". So this is not "one repository is missing", it is a parameter modelled as a constant, with the whole draft/publish/revisions/restore lifecycle already written and repository-agnostic underneath it.Only two of the eight are documented. All eight have exactly 30 mentions in the spec, which is to say they appear only as enum values: there is no content schema for any of them, because
entriesis an opaque flat object. Roblox's configs guide has a "Use case | Repository" table with one row, and says repositories "will eventually expand".DataStoresConfigis documented on a different page. The enum's own description explains the rest: "internal repository types are not exposed to allow development and testing before enabling". They are forward declarations, so this PR carries the transport and takes no view on what any repository's entries mean.That is also why there is no bespoke command per repository: a generic
--repositoryworks the day Roblox documentsLeaderboardsConfig, with no release of this tool.Where the client went, and why
rbx_core::api::configs, for the reasonsend_with_csrflives there: four crates carried their own copy until they started disagreeing about whether204was a success. In this workspace leaf tools depend onrbx-corealone and only aggregators (rbx-check,rbx-import,rbx-doctor) depend on peers, so a second tool reaching the Configs API throughrbx-configwould have been the first peer-to-peer edge.rbx configgains--repository, andrbxconfig.tomlan optionalrepositoryfield which is the source of truth for the commands that read it. A flag contradicting the file is refused, naming both:Every existing invocation is unchanged: no flag and no field means
InExperienceConfig, andrbx config initwrites the file it always did.Two bugs in that code
previousDraftHashwas threaded through and hardcoded off.overwrite_drafttook the parameter;sync_and_publishpassedNone. Roblox documents it as optimistic concurrency: the request fails when the hash does not match the server's draft. Sorbx config syncsilently discarded a draft somebody had staged in the Creator Hub, and the draft was gone before they could learn it existed. The draft is now read first, its hash sent, and what was replaced is named on the way past. Not a refusal: pipelines legitimately overwrite, and a hard stop would break them.The documented limits were not checked locally. 100 keys per repository, 256 characters per key. The 101st key failed as a 400 mid-publish.
--dry-runrefuses it too now, because a dry run reporting a clean plan for a publish that cannot happen is the wrong answer. Key length countschars, not bytes, since the guide says "256 characters" and a byte count would refuse 200 accented characters Roblox accepts.rbx rtbf
DataStoresConfigholds the right-to-be-forgotten deletion templates.rbx config --repository DataStoresConfigcan push them today, and that is exactly the problem: its entry model holds an opaquetoml::Value, and every mistake in this file is one Roblox accepts and then silently never matches.{UserId}is case-sensitive.{userId}is stored happily and deletes nothing. This is the first item on Roblox's own best-practices list.global, which is wrong if your keys are scoped.None of those produce an error from Roblox. You find out when a legal request goes unfulfilled, and the guidance is to compare patterns against your Luau by hand in the Creator Hub and confirm within 30 days that the data went, which is an admission that nothing verifies it for you.
rbx rtbf verifyanswers the questioncheckcannot: does each template name a data store that exists. Both can agree perfectly on a template naming a store you renamed last year. A store pattern is matched by requiring digits where the token is, soPlayer_{UserId}_Savedoes not matchPlayer_Settings_Save: a verify that says yes too easily is worse than none. Ordered stores are reported as unchecked rather than missing, becauseCloud_ListDataStoresdoes not list them and a false alarm teaches people to ignore the command.No lockfile, for the reason
rbx confighas none: the published config is readable in full, so the remote state is a fetch. Not per env either, because a key naming scheme belongs to a codebase rather than an environment, so--env allpublishes the same declaration to each universe.Wiring, and the enumerations it turned up
Wired the way every declarative tool here is: the binary,
rbx check(two rows, so the local rules still run under--offline, and a missing key isSkippednotErrorfollowing whattools::configrecords), the JSON schema,docs/rtbf.md,SUMMARY.md, README,docs/check.md.Three enumerations that a new tool has to join and that nothing links together:
crates/rbx/tests/cli_smoke.rsSUBCOMMANDS, which drives the help coverage tests.crates/rbx-doctor/src/coverage.rsREQUIREMENTS, the per-config-file scope table.rbxrtbf.tomlwas missing it, sorbx doctoranswered nothing about rtbf coverage. Added, and the "none of ... are here" sentence is now derived from the table instead of restating the file names.rbxrtbf.tomlhas none, so that claim is corrected.rbx importis the fourth and is not done: adding aDomain::Rtbftouches recorded wiremock fixtures I cannot re-record offline, and whether adopting a universe should adopt its deletion templates is a decision rather than a mechanical addition.docs/import.mdsays so under "What it cannot import", with the hazard stated: writingrbxrtbf.tomlby hand after an import and runningsyncwould clear whatever the universe had. Pull first.Verification
Workspace suite green.
fmt,clippy --all-targetsand the rustdoc lint clean.cargo run -p rbx-schema -- --checkexits 0 withschemas/rbxrtbf.schema.jsoncommitted.Smoke-tested on the built binary:
rtbf --helplisting six subcommands,initthenshowon a fresh file,showon the worked example printing the sample keys Roblox will look for, a miscased{userId}refused with the case-sensitivity message,rbx checkreporting both rtbf rows with and without--offlineand without a key,--repository DataStoresConfigreaching the right path, an unknown repository listing the eight, the flag-versus-file contradiction refused, andsync --dry-runrefusing a 101st key.