Skip to content

Fix todo creation/editing with jQuery 4; add CI tests (re-land of #6) - #10

Open
codeling wants to merge 3 commits into
mainfrom
claude/ci-security-tests
Open

codeling wants to merge 3 commits into
mainfrom
claude/ci-security-tests

Conversation

@codeling

Copy link
Copy Markdown
Owner

Summary

Bug fix: creating and editing todos is broken on main since the jQuery 4.0.0 update (#9)

jQuery 4 no longer serializes non-plain objects passed as $.ajax data. addItem and storeItemRemote in todo.js pass Todo instances, so the browser posted the literal body [object Object]. The server then answered "Todo may not be empty!", so adding or saving an edited todo always failed. Both calls now serialize explicitly with $.param(stuff). All other ajax calls already use plain objects. Separate commit: ed62752.

CI tests (re-land of #6)

#6 was merged into its base branch claude/magical-cray-3rk097. #5 had already been merged into main the day before, so the tests never reached main. This PR re-applies that commit on top of the current main:

  • tests/api.test.js, tests/ui.test.js, tests/apache-access.sh
  • .github/workflows/tests.yml (PHP lint, integration tests against MariaDB, Apache access rules)
  • tests/ denied in .htaccess / nginx.conf.sample
  • playwright as dev dependency (npm ci && npm run vendor leaves vendor/ unchanged; npm audit --omit=dev clean)

New tests for changes merged since

  • Ownership checks from Follow-ups to the mobile rework: caching, dialogs, time zones, ownership checks, cleanup #4: lists and todos of another user can neither be read nor changed. This covers enter, update (including moving an own todo into a foreign list), complete, trash, reactivate-one, empty-trash, query-todos, query-tags and query-lists.
  • Library versions: the UI test checks that the page runs with the jQuery and jQuery UI versions from package-lock.json.
  • Edits are saved: the UI test checks that an edit made in the dialog reaches the database. This check would have caught the jQuery 4 bug.

Testing

Run locally with MariaDB 10.11, PHP 8.4, Chromium and Apache 2.4:

  • npm test: 10/10 pass, on repeated runs. Before the fix, the workflow UI test fails on current main.
  • Regressions are caught:
    • With the old reactivate-temp.php and unescaped todo rendering restored, the SQL injection test and both UI tests fail.
    • With the jQuery 4 fix reverted, the workflow UI test fails.
  • tests/apache-access.sh: all 22 checks pass.
  • php -l is clean on all PHP files.
  • The GitHub workflows run for the first time on this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XgpcjUfZKQgJmGv5zfyvb1


Generated by Claude Code

- tests/api.test.js: CSRF enforcement on all modifying endpoints, the
  normal create/update/complete/trash flow, second-order SQL injection via
  recurring-entry reactivation, automatic reactivation, input validation,
  generic database errors and security headers.
- tests/ui.test.js (Playwright): HTML stored in the database is rendered as
  text in the list, tags, list names and log; the main workflows run
  without JS errors or Content-Security-Policy violations.
- tests/apache-access.sh: .htaccess requires authentication and denies
  internal files.
- tests.yml workflow runs PHP lint, the Node tests against MariaDB and the
  Apache check. tests/ is also denied in .htaccess and nginx.conf.sample.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XgpcjUfZKQgJmGv5zfyvb1
jQuery 4 only serializes plain objects passed as ajax data; Todo instances
were sent as '[object Object]', so enter.php and update.php received no
fields ('Todo may not be empty!'). Serialize them explicitly with $.param.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XgpcjUfZKQgJmGv5zfyvb1
- API test: lists and todos of another user can neither be read nor
  changed (enter, update incl. moving into a foreign list, complete,
  trash, reactivate, empty trash, query-todos, query-tags, query-lists).
- UI test: the page runs with the jQuery/jQuery UI versions from
  package-lock.json, and an edit made in the dialog reaches the database
  (this would have caught the jQuery 4 serialization bug).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XgpcjUfZKQgJmGv5zfyvb1
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.

2 participants