golem15/summercms
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(19.6-04): complete unit tests and security review plan

Browse Source
This commit is contained in:
Jakub Zych
2026-10-26 15:34:38 +01:00
parent 07f4632dc2
commit 4925dddd40
1 changed files with 94 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

119
.planning/phases/19.6-two-factor-auth-and-device-flow-in-sm-user-plugin/19.6-04-SUMMARY.md
View File

@@ -17,24 +17,31 @@ provides:
- sm-mfa-plugin unit tests, one per decision, the PHP bodies, the login-path invariant and the PHP security test map
- sm-sociallogin-plugin TestSocialGate
- 19.6-SECURITY-REVIEW.md with the explicit D-14 section; 19.6-VALIDATION.md validated
- one post-release fix in sm-mfa-plugin (71d8f27), unreleased pending the Task 5 reply
- one post-release fix in sm-mfa-plugin (71d8f27), released as sm-mfa-plugin v0.1.1 and pinned by sm-demo-app (pushed) and sm-bm-app (committed; push waits on another session's commits)
- finding F-1 (activate-by-code) deferred with a todo
affects: [19.6 verification, sm-demo-app, sm-bm-app, fonoteka.go, sm-mfa-plugin release]
actuals:
tokens: 97673 # MEASURED chars/4 over added lines: summercms.go e95277a..692beb5 (80239), sm-user-plugin 3b22134..1f3266a (94434), sm-mfa-plugin 33ad164..d08f1ec (204699), sm-sociallogin-plugin b752177..14d5f93 (11321)
tasks: 4 # Tasks 1-4 done; Task 5 is the open checkpoint, Task 6 follows the reply
commits: 5 # MEASURED in summercms.go before this SUMMARY commit: git rev-list --count e95277a..692beb5
tasks: 6
commits: 7 # MEASURED in summercms.go before this final SUMMARY commit: git rev-list --count e95277a..HEAD (5 Task 1-4 commits, the draft summary, the Task 6 docs commit)
sm_demo_app_commits: 1 # 9166a1e
sm_bm_app_commits: 1 # fe4e292
sm_user_plugin_commits: 3 # MEASURED: git rev-list --count 3b22134..1f3266a
sm_mfa_plugin_commits: 5 # MEASURED: git rev-list --count 33ad164..d08f1ec
sm_sociallogin_plugin_commits: 1 # MEASURED: git rev-list --count b752177..14d5f93
plan_head_before: e95277a21049bf92a1f427d2d9379c4d26da35d8
plan_head_after: 692beb5
plan_head_after: see the final SUMMARY commit
sm_user_plugin_head_before: 3b22134
sm_user_plugin_head_after: 1f3266a
sm_mfa_plugin_head_before: 33ad164
sm_mfa_plugin_head_after: d08f1ec
sm_sociallogin_plugin_head_before: b752177
sm_sociallogin_plugin_head_after: 14d5f93
sm_demo_app_head_before: bb24207
sm_demo_app_head_after: 9166a1e
sm_bm_app_head_before: 4759349
sm_bm_app_commit: fe4e292
tech-stack:
added: []
@@ -48,6 +55,7 @@ key-files:
created:
- scripts/check-phase19.6.sh
- .planning/phases/19.6-two-factor-auth-and-device-flow-in-sm-user-plugin/19.6-SECURITY-REVIEW.md
- .planning/todos/pending/2026-10-26-activate-by-code-second-factor.md
- ../fonoteka.go/plugins/golem15/user/login_gate_test.go
- ../fonoteka.go/plugins/golem15/user/password_changed_test.go
- ../fonoteka.go/plugins/golem15/user/reset_two_factor_test.go
@@ -65,33 +73,37 @@ key-files:
- ../fonoteka.go/plugins/golem15/user/updates/postgres_test.go
- ../sm-mfa-plugin/{classes/config.go,models/settings.go,lang/en/lang.yaml,lang/pl/lang.yaml,README.md,harness_test.go}
- ../sm-sociallogin-plugin/README.md
- ../sm-demo-app/{plugins/golem15/mfa,go.mod,go.sum}
- ../sm-bm-app/plugins/golem15/mfa
key-decisions:
- "An empty settings list (methods or enforce groups) is an empty value and falls back to the configuration, as the README promised; the form stores it as NULL (sm-mfa-plugin 71d8f27, post-release fix)"
- "Error bodies are pinned byte for byte; multi-key success bodies by key set and fixed values, because Go maps sort keys where PHP kept insertion order (recorded, not changed)"
- "Finding F-1 (activate-by-code mints without the second factor when require_activation is off) is raised at Task 5 rather than fixed: it overturns a recorded plan 01 decision and needs a contract and release choice"
- "Finding F-1 (activate-by-code mints without the second factor when require_activation is off) was raised at Task 5 and deferred by the user with a todo; the accepted-debt reason is corrected in the security review"
- "Task 5 reply: release sm-mfa-plugin v0.1.1 with demo and bm pointer bumps, defer F-1, push the test-only commits; no tags for the user or social plugins"
patterns-established:
- "check-phase<N>.sh stage per repository, names appended per plan, docker NOT RUN explicit"
requirements-completed: [] # plan not complete: Task 5 (checkpoint) and Task 6 are open
requirements-completed: [SC-4, D-02, D-05, D-06, D-07, D-08, D-09, D-10, D-11, D-12, D-12a, D-12b, D-12c, D-13, D-14, D-15, D-16, D-17, D-18, D-19]
duration: about 76 min (2026-10-26 12:50Z to 14:06Z), to the Task 5 checkpoint
duration: about 76 min to the Task 5 checkpoint (12:50Z to 14:06Z), then about 20 min for Task 6 (14:15Z to 14:35Z)
completed: 2026-10-26
status: draft
status: complete
---
# Phase 19.6 Plan 04: Unit tests and security review Summary
**Status: draft (Task 5 checkpoint).** Tasks 1-4 are done and committed; the whole gate passes across seven repositories with Docker. One post-release fix (sm-mfa-plugin `71d8f27`) waits for the Task 5 go-ahead, and finding F-1 needs a decision. Nothing was pushed or tagged.
**Status: complete.** All 6 tasks are done. The whole gate passes across seven repositories with Docker. The one post-release fix (sm-mfa-plugin `71d8f27`) shipped as **sm-mfa-plugin v0.1.1**. sm-demo-app is pinned to it and pushed. sm-bm-app is pinned in a local commit; its push waits because its unpushed range also holds another session's phase 03.2 commits. Finding F-1 is deferred with a todo.
**One fail-closed gate over seven repositories, 41 new named unit tests across sm-user-plugin, sm-mfa-plugin and sm-sociallogin-plugin (one per decision, the PHP bodies, the token-mint inventory and every login path), the PHP security test mapped, a security review with the D-14 passkey section, one real defect fixed (an untouched settings form disabled every second factor) and one finding raised (activate-by-code).**
## Performance
- **Started:** 2026-10-26T12:50Z
- **Checkpoint:** 2026-10-26T14:06Z
- **Tasks:** 4 of 6 done; Task 5 is the open checkpoint
- **Checkpoint (Task 5):** 2026-10-26T14:06Z
- **Task 6:** 2026-10-26T14:15Z to 14:35Z
- **Tasks:** 6 of 6
## Commits per repository (in order)
@@ -129,7 +141,43 @@ status: draft
|------|--------|---------|
| 3 | `14d5f93` | test: social sign-in gate |
No push, tag or remote operation. sm-demo-app, sm-bm-app and fonoteka.go have no commit from this plan (fonoteka.go's working tree shows ` M plugins/golem15/user` because its user submodule now sits on the three test commits; the committed pointer is still `3b22134`).
**Task 6 (applications):**
| Repository | Commit | Message |
|------------|--------|---------|
| sm-demo-app | `9166a1e` | chore: bump sm-mfa-plugin to v0.1.1 (gitlink `plugins/golem15/mfa` to `d08f1ec`, go.mod require v0.1.1, go.sum tidied: only the mfa lines changed) |
| sm-bm-app | `fe4e292` | chore: bump sm-mfa-plugin to v0.1.1 (gitlink only; bm uses a replace) |
fonoteka.go has no commit from this plan. The user plugin has no release, so its pointer needs no bump. fonoteka.go's working tree shows ` M plugins/golem15/user`, because the submodule sits on the three pushed test commits while the committed pointer is still `3b22134`. fonoteka.go was not pushed.
## Remote operations (in order)
Before every push I ran `git fetch` and checked that `origin/master..master` held only this plan's commits and that nothing was behind. Nothing was force-pushed or deployed.
| Repository | Ref | Before → after |
|------------|-----|----------------|
| sm-user-plugin | `refs/heads/master` | `3b22134` → `1f3266a417777bc0fcf3b7ea3578d0d053150e9f` (3 test commits; no tag) |
| sm-mfa-plugin | `refs/heads/master` | `33ad164` → `d08f1ec430686ac99da9c30ca85a56fc6e80995e` (5 commits) |
| sm-mfa-plugin | `refs/tags/v0.1.1` (annotated) | new: tag object `c461c61cab41bda3ec097413fd79e5a2371fcfb1` → commit `d08f1ec430686ac99da9c30ca85a56fc6e80995e`; the message names the modes disabled, optional and enforced |
| sm-sociallogin-plugin | `refs/heads/master` | `b752177` → `14d5f93302082d5b94b574d6c7c24e5e60dbd902` (1 test commit; no tag) |
| sm-demo-app | `refs/heads/master` | `bb24207` → `9166a1ec5be5b689b766271214523d00a07b125d` (exactly the bump commit; the leftover journal and oauthserver moves stay unstaged) |
| sm-bm-app | not pushed | `origin/master` is still `ebb2266`. `origin/master..master` holds 7 commits: 6 phase 03.2 planning commits of another session (`17e8bf8`, `a5e1d9a`, `a16ba1d`, `009ff98`, `4759349`, then `8733060` made on top of my bump while I worked) plus my `fe4e292`. Pushing master would publish that session's commits, so I left it for the user or that session. |
`ls-remote` confirms the four branch heads and the tag above.
## merge-base results (fix `71d8f27` against every pin, in sm-mfa-plugin)
| Pin | Commit | `merge-base --is-ancestor 71d8f27 <pin>` |
|-----|--------|----------------------------------------|
| tag v0.1.1 | `d08f1ec` | 0 |
| sm-demo-app `plugins/golem15/mfa` gitlink at origin/master `9166a1e` | `d08f1ec` | 0 |
| sm-demo-app go.mod require v0.1.1 | `d08f1ec` | 0 |
| sm-bm-app `plugins/golem15/mfa` gitlink at master (`fe4e292`, also in HEAD `8733060`) | `d08f1ec` | 0 |
Task 6 verify at the final pins:
- `--demo`, `--bm` and `--fonoteka` pass.
- sm-demo-app also passes a module-mode release build (`GOWORK=off go build -trimpath -tags summercms_mcp`, which fetches sm-mfa-plugin v0.1.1 anonymously, now that the repository is public) and `go test -short ./...`.
- sm-bm-app passes `go test -short ./...`.
## Tests added (names the gate requires)
@@ -175,16 +223,21 @@ Production commits this plan made after plan 03's release (outside `*_test.go`,
|------------|--------|-------|----------------------------|
| sm-mfa-plugin | `71d8f27` | `classes/config.go`, `models/settings.go`, `lang/en/lang.yaml`, `lang/pl/lang.yaml`, `README.md` | tag `v0.1.0` (`33ad164`); sm-demo-app gitlink `plugins/golem15/mfa` (`33ad164`) and its go.mod require `sm-mfa-plugin v0.1.0`; sm-bm-app gitlink `plugins/golem15/mfa` (`33ad164`) |
`git -C ../sm-mfa-plugin merge-base --is-ancestor 71d8f27 33ad164` exits 1: no pin contains the fix. sm-user-plugin and sm-sociallogin-plugin have test and README commits only: nothing to release there (their pins stay correct). Before the reply, no new tag exists: `git -C ../sm-mfa-plugin tag --points-at HEAD` is empty.
At the checkpoint, `merge-base --is-ancestor 71d8f27 33ad164` exited 1 (no pin contained the fix) and `tag --points-at HEAD` was empty. sm-user-plugin and sm-sociallogin-plugin had test and README commits only, so there was nothing to release there. After Task 6 every pin contains the fix (table above).
## Task 5 reply
Pending: the checkpoint was returned to the orchestrator. Plan 03's Task 4 reply ("1. release 2. push demo 3. you can sudo chown golem:golem sm-sociallogin-plugin if needed.") did not pre-authorise plan 04 patch releases.
Plan 03's Task 4 reply did not pre-authorise patch releases for this plan, so the checkpoint went to the user. The user's decision, relayed verbatim by the orchestrator (from AskUserQuestion):
Decisions asked at Task 5:
1. Release the sm-mfa-plugin fix (proposed tag v0.1.1 on the commit after the release, with the corrected mode names in the tag message), then the demo pointer, go.mod require and go.sum, and the bm gitlink in separate commits, pushed after the usual fetch and range check; or hold (todo plus g15office task).
2. Finding F-1: (a) fix activate-by-code in sm-user-plugin with a patch release, (b) accept with the corrected reason, or (c) defer to a todo.
3. Whether to push the test-only commits of sm-user-plugin (3), sm-mfa-plugin (4) and sm-sociallogin-plugin (1) along with the release.
> 1. RELEASE. Tag sm-mfa-plugin v0.1.1 on its current HEAD, which includes 71d8f27 and the test commits. The tag message names the modes correctly: disabled/optional/enforced. Push master and the tag. Then, in separate commits:
> - sm-demo-app: bump plugins/golem15/mfa to that commit, update go.mod's require to v0.1.1, and go.sum.
> - sm-bm-app: bump the plugins/golem15/mfa gitlink.
> Fetch first and check the range before pushing both. In sm-demo-app, leave the unstaged journal/oauthserver pointer moves alone. Mark the deferred tag-message item resolved.
> 2. F-1: DEFER. Write a todo in .planning/todos/pending/ and record F-1 as deferred in the security review, with the corrected reasoning. I'll mirror it to g15office.
> 3. YES, push the test-only commits on master: sm-user-plugin (3), sm-mfa-plugin (part of item 1) and sm-sociallogin-plugin (1). No new tags for user or social. Never push fonoteka.go.
> No sudo, no force-push and no deploy. Then do Task 6, finalize the SUMMARY and commit it. In the report, list every pushed ref and tag with its SHA.
Carried out as written, with one exception. The fetch and range check before pushing sm-bm-app found commits of another session, so sm-bm-app is committed but not pushed (Remote operations).
## Accomplishments
@@ -200,7 +253,7 @@ Decisions asked at Task 5:
**2. [Rule 3 - Blocking] updates TestMain short detection (sm-user-plugin `2c68979`).** Test-only; needed so the userplugin stage's `-short` run is Docker-free as the gate's NOT RUN rule assumes.
**3. [Rule 4 - raised, not fixed] Finding F-1, activate-by-code.** Reproduced through the mfa harness (`require_activation: false`, `activate_mode: user`: password login, TOTP enabled, activate-by-code answers a token without a challenge). Plan 01 accepted this path because "a not-yet-activated account cannot have 2FA yet", which is false in that configuration. Fixing it changes a recorded decision and an endpoint's answer and needs a user-plugin release, so it goes to the Task 5 decision (security review "Open findings").
**3. [Rule 4 - raised, deferred] Finding F-1, activate-by-code.** Reproduced through the mfa harness (`require_activation: false`, `activate_mode: user`: password login, TOTP enabled, activate-by-code answers a token without a challenge). Plan 01 accepted this path because "a not-yet-activated account cannot have 2FA yet", which is false in that configuration. Fixing it changes a recorded decision and an endpoint's answer and needs a user-plugin release, so it went to the Task 5 decision. The user deferred it: todo `2026-10-26-activate-by-code-second-factor.md`, security review "Deferred findings" with the corrected reason.
**4. [Carry-forward] D-07 follows the code.** `TestPasswordChangedSources` listens on `AccessRevokedEvent` and checks `PasswordChanged()` (no `PasswordChangedEvent` exists). The administrator's save fires from `FormBeforeUpdate`, before cabana writes the record in the same transaction, so a listener reads the old hash there; the test pins that and proves the transaction with a marker write that commits or rolls back with the password.
@@ -222,7 +275,9 @@ Decisions asked at Task 5:
**13. [Orchestrator update] sm-mfa-plugin is public.** The gate needs no credential note; deferred-items.md marks the private-repository item resolved.
**Total deviations:** 13 (1 bug fixed in production, 1 test-harness fix, 1 finding raised, 2 carry-forwards, 1 plan-text mismatch, 3 adapted assertions, 1 test design, 1 scope, 2 environment).
**14. [Range check, Task 6] sm-bm-app not pushed.** Its unpushed range includes six phase 03.2 planning commits of another session (one made on top of my bump while I worked). The reply asked for a range check before pushing, and that check failed, so the bump `fe4e292` is committed but unpushed. To publish it: `git -C sm-bm-app push origin master`, once that session's commits may go out too.
**Total deviations:** 14 (1 bug fixed in production and released, 1 test-harness fix, 1 finding raised and deferred, 2 carry-forwards, 1 plan-text mismatch, 3 adapted assertions, 1 test design, 1 scope, 2 environment, 1 push held back by the range check).
## Known Stubs
@@ -234,17 +289,31 @@ None: plan 04 adds tests, a script and documents; the one production change narr
## Issues and follow-ups
- **F-1** (above): decision at Task 5.
- **deferred-items.md** gains: the `classes` package of sm-user-plugin starts Postgres under `-short` (predates the phase; same fix as `2c68979`); the JSON key-order observation; F-1. The private-repository item is resolved.
- **fonoteka.go** user submodule sits three test commits ahead of its committed pointer (no release needed; bump only if wanted).
- **F-1:** deferred; todo `.planning/todos/pending/2026-10-26-activate-by-code-second-factor.md` (the user mirrors it to g15office). The corrected reason is in the security review "Deferred findings".
- **sm-bm-app push:** pending, see deviation 14.
- **deferred-items.md** gains three items:
- the `classes` package of sm-user-plugin starts Postgres under `-short` (predates the phase; same fix as `2c68979`);
- the JSON key-order observation;
- F-1, deferred.
It marks two items resolved: the private repository (made public) and the v0.1.0 tag message (v0.1.1 names the modes correctly).
- **fonoteka.go:** the user submodule sits three pushed test commits ahead of its committed pointer. No release is needed; bump it only if wanted.
- **Changelog:** CLAUDE.md asks for a summercms.io changelog post per release tag of the framework (`vX.Y.Z` of summercms.go). v0.1.1 is a plugin tag, so no post was made. The user decides if one is wanted.
## g15office
This executor has no g15office tool; the orchestrator posts the plan 04 update on milestone task `avoroeb4` (gate result: all 11 stages PASS with Docker).
This executor has no g15office tool. The orchestrator posts the plan 04 update on milestone task `avoroeb4`: gate all 11 stages PASS with Docker; sm-mfa-plugin v0.1.1 released; F-1 deferred (the user mirrors the todo).
## Self-Check: PASSED
- Commits exist: summercms.go `947009a`, `e9785f7`, `531e7ab`, `ea4680e`, `692beb5`; sm-user-plugin `6844d36`, `2c68979`, `1f3266a`; sm-mfa-plugin `71d8f27`, `8ec140e`, `2aff160`, `ab886f8`, `d08f1ec`; sm-sociallogin-plugin `14d5f93`.
- Files exist: `scripts/check-phase19.6.sh` (executable), `19.6-SECURITY-REVIEW.md`, the test files listed in key-files, `../sm-mfa-plugin/testdata/PHP_METHOD_MAP.md` and `php_methods.txt`.
- Acceptance greps of Tasks 1-4 pass (gate: `PHASE196_REQUIRE_DOCKER` 7, mode lines 12, `TestWebAuthnCeremonySmoke` 2, `202610210002_create_device_auth_sessions` 2; review: T-19.6 references 44, `D-14` 4; validation: `nyquist_compliant: true` 1, `check-phase19.6.sh` 10).
- Working trees: summercms.go keeps only other sessions' `STATE.md`, `state.json` and untracked paths; the three plugin repositories are clean.
- Task 6:
- Commits `9166a1e` (sm-demo-app) and `fe4e292` (sm-bm-app) exist.
- The tag v0.1.1 and every pushed head match `ls-remote`.
- The todo file exists.
- Working trees:
- summercms.go keeps only other sessions' `STATE.md`, `state.json` and untracked paths.
- The three plugin repositories are clean and even with origin.
- sm-demo-app keeps only the leftover journal and oauthserver moves.