Better discipline in constructing verificationtarget
for verifying comms channels. It's still not perfect and
there's still email-defaultism but I want this bandaid in
before we do more work later on to make email not special.
Better hygiene here also for secrecy vs not secret notifs.
More type upgrades to come after this!
Lewis: May this revision serve well! <did:plc:3fwecdnvtcscjnrx2p4n7alz>
This endpoint was lacking rate limits, even though it can be a very costly operation. I set it to a 5 per 60s per ip rate limit, which should support normal behaviors and mitigate any misbehaving relays.
The spec allows for a maximum of 2MB of total record block bytes in a commit event. In indigo-relay, a commit between 2MB and 2MiB results in the commit being silently dropped. Over 2MiB the commit is rejected, the connection closed and then immediately re-opened and the same commit read again. This leads to the relays essentially DDoSing the server by reading the same commit over and over again.
Once indigo-relay implements backoff, the effect will instead be that the relay can't progress and your account goes dark.
Left an explanation in the code as well but basically although the spec doesn't set a hard limit on the size of a CBOR record, there is a hard limit on individual commit event record blocks in the sync 1.1 spec. indigo-relay doesn't seem to validate the size of your commits, but hits a hard limit in the websocket library if the commit is too large, causing the connection to drop and instantly reconnect.
I had a 10MiB record on my PDS and I had 9 relays hammering my server constantly trying to backfill that same event and choking on it over and over, until I cleaned it up.
With this PR tranquil introduces a 1MB limit on CBOR records, ensuring we never go over the sync commit event limits.
As part of a subscriber getting caught up, we look up the oldest repo_seq row since the cursor they provided, reading out lots of data from the row, then throwing it all away and just looking at `created_at`. As happened to my poor baby PDS, relays can be pretty aggressive with trying to backfill and will keep triggering these lookups with no backoff, so it's worth avoiding the extra work here and limiting the query to get the created_at field only. That's about a 600x reduction in the number of bytes this lookup represents based on my PDS's data, from ~5K to 8 bytes.
I have not done the same optimization for metastore, I don't think it's as affected. The ROI is lower and the effort is higher. Might be a fun thing for someone to add if they wanted to.
Typing a struct for enqueuing messages out to comms.
Also, after recent PRs showing me the way in which the
previously unused metadata field would possibly be used going
forward, I think I deem it too much of a hazard to be
left alive, hence removing it and migrating any inflight
comms to extract html_body (only existing use for it).
Lewis: May this revision serve well! <did:plc:3fwecdnvtcscjnrx2p4n7alz>
- use tranquil_pds not comms since the validate email and templates are reexported from pds package
- implement handlebars syntax
- update templates to be transport agnostic, email, discord, telegram with handlebar examples with conditions
- refactor template and notice logic from PR #269, and #299 to be self contained enforcing all substitutions to be defined within the enum properties
- fix open TODOs, store.rs uses get_string within the ids.rs get_built_ins method
- UTC logging for User facing notices restored
- new api error for verification channel requires bot
- fix tests to use metadata for html body...
- remove unwrap and handle template rendering error explicitly
- update verify_link to use handlebars trusted url syntax {{{verify_link}}}
- fix verification enqueueing to include channel in template rendering
- use locale enum for validation and display instead of string array separately maintained
- support text only templates for signal and discord
- fix missing verify_link and verify_page in migration and signup verification templates with unit tests
- consolidated body and subject notice methods into a single rendered function
We were just using them for S3 support. This PR switches us over to the s3 crate which works the same way, with some minor differences.
The main implication of this change is that we're no longer doing all the AWS stuff, like checking aws profile in home directory, or automatic discovery of metadata.
This change is covered by our brand new s3 test suite, but I also did the old school `just test-full` with minio.
Introduces s3s and s3s-fs to run an in-memory s3 server in tests. With this we can promote s3 tests to run in `just test`, they won't need containers, and they're super fast. Adds `just test-s3` to run them more easily.
```
Nextest run ID db9a9530-738e-4c92-ad3a-23289252cac2 with nextest profile: default Starting 6 tests across 1 binary PASS [ 0.356s] tranquil-storage::s3 put_stream_error_aborts_upload PASS [ 0.356s] tranquil-storage::s3 put_stream_empty_aborts_upload PASS [ 0.358s] tranquil-storage::s3 copy PASS [ 0.360s] tranquil-storage::s3 put_get_head_delete PASS [ 0.360s] tranquil-storage::s3 put_stream PASS [ 0.447s] tranquil-storage::s3 put_stream_multipart ──────────── Summary [ 0.448s] 6 tests run: 6 passed, 0 skipped
```
Also optimizes more hashing functions because it matters in tests.
We've got a bunch of tests that never pass because they time out on their own, or that take 10s because they test a 10s timeout, that we can rethink. Together they introduce a lot of slowness.
This puts anything over 10s in CI into test jail.
All of the codebase uses ring explicitly or implicitly, but the HTTP client used for S3 requests was using aws-lc instead. This commit removes that extra dependency.
Anecdotally it shaves 10s off a dev build on my machine, but slower machines would benefit more from it.
Overall `just test` runs about 10-20s faster on this branch than current main, for me, with around 170s instead of around 180-190s. But the cool thing is the compile time. Making a simple edit to any file involved and running `just test` on main takes about 60s to recompile the test binaries. On this branch it takes less than 15s.
This is done by replacing a ton of separate test binaries with a single megabinary.
Only test files that looked safe have been included, avoiding ones that do stuff like setting env.
Replace sleeps with the existing polling function instead. Exposes that the stress test isn't much of a stress test, it probably needs more data or something, but if it's heavy it should maybe not be part of `just test`
`just test` doesn't use docker, but the cleanup was still pruning containers over and over. This switches to only cleaning up containers created by the tests.
Flushing to disk all the time ensures no partial states when writing to disk during awkwardnesses like crashes in prod risking real user data. That's good!
We're also doing it while running tests. Less good! Well, at least not important!
Comparing a clean test run (`just test --no-fail-fast`) and a run on this branch, on my MacOS M4 Pro, I get:
`**Summary** [ 517.723s] **1424** tests run: **1405** **passed** (**7** **slow**), **19** **failed**, **15** **skipped**`
before and
`**Summary** [ 326.369s] **1424** tests run: **1405** **passed**, **19** **failed**, **15** **skipped**`
after.
The reason I run `--no-fail-fast` is that a bunch of tests always fail and time out and stuff
`rpc:*?aud=did:web:api.bsky.app#bsky_appview` fails in the permissions check even though it's a valid scope. This makes it resolve correctly.
Also this commit fixes a bug where `rpc` as a bare scope is interpreted as all methods for all services, but it's really an invalid scope. Can drop it if it's a problem, but it just looks like a bug in the implementation rather than something intended?
Note that the parser treats `rpc:*` as "allow everything for no services", but aud is actually required, so it's not a valid scope? Didn't mess with it though because delegation uses it and that part of the code treats it as allow everything for every service. We should maybe split the scope parsers so we have one for delegated scopes and one for regular?
However, to avoid the same bug as bare `rpc`, I changed the enforcement step in "regular" OAuth to disallow `rpc:*` (this matches the existing behavior on main, no actual change, just a little acknowledgement).
My last PR introduced a migration to blobs, changing them from being unique per cid to being unique per cid+user. This is because two users can upload the same blob, get the same cid, and then previously in tranquil the second user would never actually get their ownership recorded, meaning listBlobs was missing the blob, and a migration _off_ of tranquil would leave them sans blob https://tangled.org/tranquil.farm/tranquil-pds/pulls/261/round/3
This PR attempts to add a job to repair existing instances where blobs have been deduplicated and not attributed to each owning user. It does this by pulling all records for each user, and then in batches crawling the records to find blob references. For each blob reference, it checks whether that user has its ownership recorded, and if not, repairs it by adding the entry.
To avoid re-running this potentially heavy job on every startup, I've used the config table to store a little record of whether it has been run.
LewisandTangled
Johanna LarssonandTangled
SteffoandTangled
Natalie BridgersandTangled
straiforosandTangled
teqandTangled
Louis EscherandTangled
Anastasia CalicoandTangled
TrezyandTangled