Skip to content

fix: backlog cleanup after team-foundation - #10

Merged
TOMOKI977 merged 4 commits into
mainfrom
fix/backlog-cleanup
Sep 24, 2026
Merged

TOMOKI977 merged 4 commits into
mainfrom
fix/backlog-cleanup

Conversation

@TOMOKI977

Copy link
Copy Markdown
Contributor

Summary

Small fixes found while taking team-foundation to production, so change 2 (GitHub alerts) starts from a clean base.

  • Keyring docs: the quick path in docs/key-backup.md now generates the full JSON keyring. A bare base64 key is what caused the production 500.
  • Diagnosable config errors: composition failures log a fixed, non-sensitive reason for config errors (PII_KEYRING, BOT_INFO) through a new ConfigError. Other errors still log only their name. Secret values never reach the log.
  • BOT_INFO validation: rejects bad JSON and wrong shapes (null, array, missing numeric id or string username). The production value passes.
  • Non-object webhook bodies: a valid-JSON body such as null crashed the handler into a 500, so Telegram retried it forever. It is now logged as MalformedUpdate and answered 200, matching the existing malformed-body policy.
  • /setup refusals: in a DM it asks to run it inside the group, and anonymous admins are told to turn off "Remain anonymous".
  • Team picker: callback data goes through a new parseTeamId instead of an unchecked cast. ids.ts is the single source for the team-id pattern.
  • wrangler.jsonc: workers_dev and preview_urls are 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:

  • The team-id regex was duplicated between ids.ts and team-picker.ts.
  • BOT_INFO was only checked for JSON syntax, not for shape.
  • The accept-side boundaries of parseTeamId (1 and 64 characters) were not tested.

Tracked for later: parseKeyRing is 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 errors
  • wrangler deploy --dry-run accepts the config

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.
@TOMOKI977 TOMOKI977 added the size:exception PR over the 400-line review budget, accepted explicitly label Sep 24, 2026
@TOMOKI977
TOMOKI977 merged commit c60e089 into main Sep 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:exception PR over the 400-line review budget, accepted explicitly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant