fix: backlog cleanup after team-foundation - #10
Merged
Merged
Conversation
A raw base64 key as PII_KEYRING fails parseKeyRing and turns every webhook call into a 500.
Composition failures now log a fixed, non-sensitive reason for config errors (PII_KEYRING, BOT_INFO shape) instead of only the error name. A valid-JSON but non-object webhook body is logged as MalformedUpdate and answered 200 instead of crashing into a 500 retry loop.
/setup in a DM asks to run it inside the group, and anonymous admins are told to disable 'Remain anonymous'. Team picker callback data goes through parseTeamId instead of an unchecked cast.
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.
Summary
Small fixes found while taking
team-foundationto production, so change 2 (GitHub alerts) starts from a clean base.docs/key-backup.mdnow generates the full JSON keyring. A bare base64 key is what caused the production 500.reasonfor config errors (PII_KEYRING,BOT_INFO) through a newConfigError. Other errors still log only their name. Secret values never reach the log.BOT_INFOvalidation: rejects bad JSON and wrong shapes (null, array, missing numericidor stringusername). The production value passes.nullcrashed the handler into a 500, so Telegram retried it forever. It is now logged asMalformedUpdateand answered 200, matching the existing malformed-body policy./setuprefusals: in a DM it asks to run it inside the group, and anonymous admins are told to turn off "Remain anonymous".parseTeamIdinstead of an unchecked cast.ids.tsis the single source for the team-id pattern.wrangler.jsonc:workers_devandpreview_urlsare explicit. Both were already on by default, so deployed behavior does not change.Size exception: about 440 lines, most of them malformed-input tests.
Review
Full review with four lenses, since this touches secret handling and the webhook route. No blocker or critical findings. The three warnings were fixed in this PR:
ids.tsandteam-picker.ts.BOT_INFOwas only checked for JSON syntax, not for shape.parseTeamId(1 and 64 characters) were not tested.Tracked for later:
parseKeyRingis more lenient than its "fail closed" comment claims (it accepts trailing base64 junk and loose key versions). It was not tightened here, because that could reject the secret production runs with. Check the live secret's format first.Test plan
npx vitest run: 197/197 tests pass (144 before)npx tsc --noEmit: no errorswrangler deploy --dry-runaccepts the config