Release: develop -> main - #4557
Merged
Merged
Conversation
`docs/cron-jobs.md` lists every `@DfxCron` declaration with its interval, its process flag and its location: 131 jobs across 92 files and 33 areas. The list is derived by a balanced-paren scan of the decorator arguments, so multi-line declarations are included — a line-based match misses four of them. The parsed count is asserted against a raw text count of the decorator. Two things the list makes visible: - 21 jobs carry no process flag and therefore run unconditionally. Three of those are deliberate: the `ProcessService::resync*` jobs maintain the disabled set and the JWT denylists themselves, so making them switchable would let a configuration change disable the mechanism that reads configuration changes. The remaining 18 are an omission and are listed individually. - `CitreaBaseStrategy::checkPayInEntries` is declared on an abstract class. NestJS discovers providers rather than classes, so such a job is registered once per concrete subclass. There is one subclass today, so the runtime count still equals the declaration count. Documentation only. No source file is touched.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
August 1, 2026 00:37
* Record client-side errors as ERROR log lines Frontend errors were visible only in the customer browser console, so a failure that many customers hit left no trace in log monitoring at all. Adds POST /v1/log/clientError, which writes the reported error as an ERROR line through DfxLogger. Unauthenticated on purpose: the errors worth catching happen before or without a session. Rate limited per client, with every field length-capped. Because the endpoint is unauthenticated, every field is attacker-controlled. Sensitive query parameters are redacted and line breaks are collapsed, so a forged payload can neither leak credentials into the log nor open a line that reads like a log entry of its own. * Harden the client error log against forged and sensitive input Redaction was anchored on a leading ? or &, which only holds if the value is a well-formed query string. message and stack are free text, so a credential reached the log unredacted whenever it sat at the start of the value, behind a fragment, behind a matrix parameter, inside a percent-encoded URL, or in a compound parameter name such as accessToken. Now matched on a word boundary with the name as a substring, and = or its percent-encoded form. The free text also sat in front of the key=value context using the same syntax, so a payload could forge context fields without needing a line break at all - collapsing line breaks did not prevent that. Context now comes first and every value is embedded as a quoted string, which also turns control characters and ANSI escapes into escape sequences instead of letting them repaint a terminal that tails the log. Adds a process-wide budget: per-client throttling bounds one caller, but nothing bounded a distributed flood of the ERROR stream that alerting reads. Over budget, reports are dropped and the gap is recorded rather than left silent. Also reads the header through the shared constant, trims the stack like every other string field, and pins the route mounting and rate-limit wiring in a controller spec. * Redact by stripping URL parameters instead of matching their names Matching parameter names lost a round to every new encoding: after the first pass closed the prefix-anchored gaps, a colon separator, JSON-style quoting, a double-encoded =, and a percent-encoded character inside the name itself all still carried a credential into the log. That search space has no end, so the approach goes rather than the next patch. Every URL in the text is now cut to its origin and path, which drops the query and the fragment whole. How a parameter is named, spelled or encoded stops mattering, because none of it survives. Bare assignments outside a URL are still masked, but only for names that are unambiguously secret. That second list also had to shrink. As a substring, code matched statusCode, errorCode, countryCode and currencyCode, and key matched keyboardLayout and monkey - the report would have arrived stripped of exactly the fields worth reading. Tests now pin both directions: twelve shapes a credential can arrive in, and ten diagnostic values that must survive, including the asset URL of a failed chunk. The redaction no longer claims more than it does: a secret with no recognisable name outside a URL is indistinguishable from a diagnostic string, and the comment says so. * Cut parameters from any path, not only from a URL with a scheme The parameter stripping required a literal scheme://, which left three shapes untouched: a relative path - the normal shape for an app calling its own API - a protocol-relative one, and a fully percent-encoded one. All three fell back on the name list, and that list had just been narrowed, so the gap reopened exactly where the earlier rounds had closed it. A path now qualifies on its own, and the separators are recognised percent-encoded as well. A credential can also sit in the authority (scheme://user:secret@host), where it has no separator in front of it and survived the cut untouched. That part is now dropped too. The colon separator added in the previous commit was matching secret names as substrings, which made it fire on ordinary prose: `401 Unauthorized: invalid credentials` and `Gmail: no app found` lost the half of the sentence that says what happened - and those sentences are what a frontend error usually consists of. A bare colon now requires the name to stand on its own word boundaries; the substring form stays for `name=value` and for quoted JSON keys, where prose does not collide. Tests pin all of it, in both directions: sixteen shapes a credential can arrive in, and twenty diagnostic values and sentences that have to survive. * Take the cost and the blind spots out of the assignment matching The name pattern sat in front of the separator, so it had to be retried at every position of a long word. A field of repeated word characters within the accepted length took seconds of CPU, on the one thread that serves every other request - a single post was enough to stall the process. The match is now anchored on the separator and the name read from the text in front of it, which takes the same field from seconds to well under a millisecond. A test pins it. The budget check also moved ahead of the sanitizing, so a request that will be dropped no longer pays for work nobody reads. Splitting the separators in the previous commit had cost the colon form its compound names: accessToken, sessionId, refreshToken and apiSecret all passed through unmasked, which is exactly the shape a frontend produces when it puts a caught error into a message. A colon now matches a secret name that is a part of the name in its own right, so those are covered while Unauthorized and Gmail still are not. auth is gone from the list and spelled out as authorization: as a name part it means authentication rather than a credential, and it would have taken authMethod with it. And a value is now ended by whitespace alone - a comma inside a password used to leave the rest of it in the log. The comment no longer claims the redaction covers every name: outside a URL it depends on recognising one, and it now says so. * Stop one assignment from swallowing the secret behind it Ending a value at whitespace alone was the wrong half of the trade. A harmless assignment matched first, its value ate everything up to the next space - and because a global replace never re-reads what a match consumed, the assignment behind it was never examined. debug: a=1;password=<secret> went into the log untouched, and so did the same shape from a serialised form, a referrer, or a comma-joined context dump. Those are ordinary frontend error payloads, not crafted ones. A value now ends at any joiner. The cost is the tail of a value that legitimately contains one: a password with a comma in it is masked in part rather than in full. That is the safe direction of the two, and it is pinned by a test so it does not get "fixed" back into swallowing. The comment claimed the gap was limited to names the code does not know. It now also states that masking ends where the value ends, even under a name it does know. * Type the segment list so the build stops inferring never String.match returns null when nothing matches, so the fallback made the type a union of RegExpMatchArray and an empty array literal, and the compiler settled on never for the element. It surfaced only in CI, where the compile runs through ts-jest over the whole project rather than the file alone.
TaprootFreak
approved these changes
Aug 1, 2026
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
1 participant
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist