Repository navigation
fix: read the caller's team, and carry two ReDoS patches - #137
Merged
Merged
Conversation
getTeam ignored its id and returned the first team in the store. Upstream runs one team per deployment so it never shows. DFE runs one per org, so every member's GET /me and GET /team carried the first team's name, API key and query limits. - getTeam now looks the team up by id. One expression in controllers/team.ts, catalogued in .fork-surface and docs/fork/what-we-changed.md. - /me strips apiKey and allowedAuthMethods for anyone but the engine, the same as /team. - The /team strip never worked against the real response. The router sends a mongoose document, whose fields are getters, so the filter saw $__ and _doc and passed the API key through inside _doc. Both strips now filter the serialised record. - dfe/__tests__/team-scope.int.test.ts seeds two teams against the real app and MongoDB. It fails with the old lookup, the old strip, or the /me mount removed.
Two regexes in renderChartConfig.ts take time in the square of their input, and a member controls both inputs through a stored dashboard tile. 64 KiB costs 2.2 s on the groupBy literal strip and 5.5 s on the sort-direction split. At 1 MiB the rewrites take 6 ms and 5 ms. These are TEMPORARY, so they ride as security/patches/0001 and 0002, generated into the tree by scripts/security-override.py --apply and stripped by --unapply before every upstream merge. Each patch header carries the advisory, the vector and the upstream tracker. Both rewrites give the same result as the old pattern for every input the call site can see: 0 differences over 24,887 distinct string literals from the repo's own tests and 262,222 distinct random inputs.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Upstream's /me mount line stays byte-identical, so the sync sees an added line instead of a modified one. It also stops code scanning re-reporting the existing rate-limit finding on that line as new.
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
2 participants
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.
Summary
Two fixes. Every member was reading the first team's record, and two chart-config regexes ran in quadratic time on input a member stores.
Team scoping
getTeam(id)ignoredidand returned the first team in the store. Upstream runs one team per deployment, so it never shows there. DFE runs one per org plus the platform team, soGET /meandGET /teamgave every member the first team's name, API key and query limits.controllers/team.ts:findOne({})tofindOne({ _id: id }). Catalogued in.fork-surfaceanddocs/fork/what-we-changed.md./menow stripsapiKeyandallowedAuthMethodsfor anyone but the engine, the same as/team./teamstrip never worked on the real response. The router handsres.jsona mongoose document, whose fields are getters, so the filter saw$__and_docand the API key went out inside_doc. Both strips now filter the serialised record.dfe/__tests__/team-scope.int.test.tsseeds two teams against the real app and MongoDB. It fails with the old lookup, the old strip, or the/memount removed.Quadratic regexes
security/patches/0001and0002, generated byscripts/security-override.py --apply, so the upstream merge never sees them.core/utils.ts, is NOT in this PR. Its input is the source'stimestampValueExpression, which only the engine writes in DFE, and the conflict-surface guard does not yet know about security patches on uncatalogued files.How to test on Vercel preview
N/A, non-UI change.
References