Skip to content

Validate records before storing them and compare ckeys after normalisation - #6

Merged
valzargaming merged 1 commit into
mainfrom
claude/record-validation
Nov 6, 2026
Merged

valzargaming merged 1 commit into
mainfrom
claude/record-validation

Conversation

@valzargaming

Copy link
Copy Markdown
Member

Requested by Val · project thread

Problem

Any POST with a valid token was stored exactly as sent. Reproduced: ckey=&discord= stored an empty record, and Al_ice was accepted next to an existing alice.

That matters beyond tidiness. Civilizationbot's read-only consumers (HostedVerificationSource::validate, used by every desktop/portable instance and planned as the Civ13 account link) reject the entire list when one record has a Discord id outside 17 to 20 digits, an empty key, or two keys that collide after normalisation. One bad write, for example via the technician's register <key>; <id> command which does not sanitise the key, would make verification unavailable on all of those instances.

Change

  • Inserts return 400 Bad Request with a plain-text reason when discord is not a 17 to 20 digit id or the key is empty or non-alphanumeric after normalisation. The bot already maps 400 to a "bad request, try again" message.
  • Keys are stored in normalised ckey form (lowercase, . _ - + space removed), the same rule as the bot's sanitizeInput and hosted validator.
  • The duplicate check compares existing records after the same normalisation, so legacy spellings are protected too.
  • SS14 names keep their spelling but must be printable and at most 256 bytes.
  • Deletion, token handling and lookups are unchanged. Existing test fixtures used 3-digit Discord ids and were updated to realistic ones.
  • README gains an "HTTP contract" section documenting fields, record shape and status codes, which the account-link work in Civ13 can build on.

Compatibility with Civilizationbot

The bot sends Civ13::sanitizeInput-normalised ckeys and real Discord ids, so its requests pass unchanged. Its integration call pattern was replayed against this branch and passed.

Verification

php scripts/lint.php                                   # Parsed 28 PHP files with PHP 8.3.6
./vendor/bin/phpunit --bootstrap vendor/autoload.php tests/isolated   # OK (19 tests, 80 assertions)

New tests/isolated/RecordValidationTest.php.


Generated by Claude Code

…ation

Any POST with a token used to be stored as sent, including empty
fields, Discord mentions and a second spelling of an already registered
key (alice / Al_ice). Civilizationbot's read-only consumers reject the
entire hosted list when one record has a Discord id that is not 17 to
20 digits, an empty key, or two keys that collide after normalisation,
so a single bad write could take verification down for every desktop
instance.

Inserts now return 400 Bad Request for a malformed Discord id or key,
keys are stored in normalised ckey form (the same rule as the bot's
sanitizeInput), and the duplicate check compares existing records after
the same normalisation. SS14 names keep their spelling but must be
printable and bounded. Deletion and token handling are unchanged.

The README now documents the fields, record shape and status codes the
bot depends on.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WTP2S4qKEFT8a6a6yFEL4
Copilot AI balanced review requested due to automatic review settings November 6, 2026 11:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@valzargaming valzargaming self-assigned this Nov 6, 2026

Copy link
Copy Markdown
Member Author

2026-11-06: head cab1f8e. Local: php scripts/lint.php (28 files) and ./vendor/bin/phpunit --bootstrap vendor/autoload.php tests/isolated OK (19 tests, 80 assertions). CI: https://github.com/Valgorithms/Verifier-server/actions/runs/42212461989 passed on PHP 8.2 to 8.5. Not verified: Civilizationbot's integration test against a bumped pin, live run.


Generated by Claude Code

@valzargaming
valzargaming merged commit a49a3e1 into main Nov 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants