From 7e6c42638c51d1169a1ddabe7aefa50b5dbd02f4 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 4 Nov 2026 11:14:28 +0100 Subject: [PATCH] docs(quick-261104-f4x): lagoon.Validate string and url rules --- .../261104-f4x-PLAN.md | 199 ++++++++++++++++++ .../261104-f4x-SUMMARY.md | 102 +++++++++ 2 files changed, 301 insertions(+) create mode 100644 .planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-PLAN.md create mode 100644 .planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-SUMMARY.md diff --git a/.planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-PLAN.md b/.planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-PLAN.md new file mode 100644 index 00000000..50980801 --- /dev/null +++ b/.planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-PLAN.md @@ -0,0 +1,199 @@ +--- +phase: quick-261104-f4x +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate.go + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate_test.go + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/phrasebook/lang/en/validate.yaml + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/phrasebook/lang/pl/validate.yaml + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/cabana/settings_string_url_test.go + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/README.md + - /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/docs/database/casts-and-validation.md +autonomous: true +requirements: + - quick-261104-f4x + +estimate: + tokens: 45000 + raw_tokens: 45000 + tasks: 2 + confidence: low + +must_haves: + truths: + - "Saving an admin settings model whose Rules() hold nullable|string|max:255 and nullable|url|max:500 through cabana's SettingsService.Put succeeds for valid values, empty strings and null; the lagoon: unrecognized validation rule \"string\" / \"url\" error is gone (the Settings > Journal bug)" + - "A value that is not a string under string gets \"The must be a string.\"; a value that is not a URL under url (Laravel's pattern, the urlPattern lagoon.ValidateRequest already uses) gets \"The format is invalid.\"; with the pl translator they are the Polish lagoon::validate lines" + - "string and url skip an empty or blank value (nil, nil pointer, pointer to empty string, whitespace-only string); required stays the rule that rejects emptiness, and required|string on an empty value reports only the required message" + - "On a string or url field that is not numeric/integer, min:N and max:N compare the length in characters (runes): 256 characters fail max:255 with \"The may not be greater than 255 characters.\", 255 Polish letters (510 bytes) pass" + - "Rule strings without string or url give exactly the results they gave before: every existing lagoon and cabana test passes unchanged, between keeps its existing check, and an unknown token still returns the unrecognized-rule error" + - "modules/lagoon/README.md and docs/database/casts-and-validation.md list string and url among the lagoon.Validate rules and describe their empty-value and length behaviour; the docs checker passes" + artifacts: + - path: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate.go + provides: "string and url cases in validateField, ruleString helper, min/max character-length check for string/url fields, string/url/min.string/max.string messages in validateMessage" + - path: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/phrasebook/lang/en/validate.yaml + provides: "lagoon::validate.string and lagoon::validate.url (en)" + - path: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/phrasebook/lang/pl/validate.yaml + provides: "lagoon::validate.string and lagoon::validate.url (pl)" + - path: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate_test.go + provides: "table tests: pass / fail / nullable-empty / pointer / translated / length" + - path: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/cabana/settings_string_url_test.go + provides: "TestSettingsStringURLRules (no Docker, the Put validation sequence) and TestSettingsStringURLRulesPostgres (SettingsService.Put end to end)" + key_links: + - from: "modules/cabana/settings.go SettingsService.Put (line ~267)" + to: "lagoon.Validate -> validateField string/url cases" + via: "settingRules(setting, model) + valuesForRules(model, rules) (model struct field values: string, *string, named string types)" + - from: "validateField url case" + to: "urlPattern in modules/lagoon/validate_rules.go (~line 1223)" + via: "direct reuse of the package-level regexp, no new pattern" + - from: "validateMessage" + to: "lagoon::validate.string / lagoon::validate.url and lagoon::validation.min.string / lagoon::validation.max.string" + via: "phrasebook keys with English fallbacks for a nil translator (cabana passes nil)" +--- + + +Teach `lagoon.Validate` (the model/settings validator) the Laravel rule tokens `string` and `url`, so admin settings screens whose model rules use them save again. + +Purpose: `modules/cabana/settings.go:267` validates a settings save with `lagoon.Validate` and the model's `Rules()`. The Journal settings model (and reportedly feedback and payment gateway models) uses `nullable|string|max:255` and `nullable|url|max:500`; `validateField` rejects any token it does not know before looking at the value, so Settings > Journal fails with `lagoon: unrecognized validation rule "string"`. The change is additive and generic: only rule strings that contain `string` or `url` (which errored until now) get new behaviour. Target release is framework v0.6.1 (phase 33 in progress): do not tag, push or touch sibling repos. + +Output: two framework commits (Task 1 the tokens, Task 2 the length semantics), each with its tests and doc updates, per the CLAUDE.md rule that a module API change updates its README and docs in the same commit. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/CLAUDE.md +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate.go +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate_test.go + +Working repo: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go (framework, branch master). Do not edit ../sm-journal-plugin or any other sibling repo, STATE.md or state.json. + +Facts gathered during planning (do not re-derive): +- validate.go `validateField` (lines 44-204): the token switch (54-114) sets flags or checks immediately; `default:` returns `fmt.Errorf("lagoon: unrecognized validation rule %q", tok)`. Then: nullable short-circuit `if nullable && isEmptyValue(val) && !required` (115-117); numeric range block (118-154); `if required && isEmptyValue(val)` returns the required message (155-157); then the tags loop (158-193), which SKIPS tags named required, min and max (line 163), plus the non-numeric `between` length check in bytes (176-189); then unique (194-202). +- Consequence, verified by a probe on HEAD: `min`/`max` on a field that is not numeric/integer are currently NOT enforced at all (`max:5` with a 10-character value passes), although docs/database/casts-and-validation.md line 111 says "on other fields they compare the length". This plan does not change that for rule strings without `string`/`url` (no behaviour change for existing rules); it is reported as a follow-up in the summary. +- validate_rules.go (request validator): `string` passes when `value.(string)` (line ~299); `url` passes when it is a string and `urlPattern.MatchString(s)` (line ~301); `urlPattern` is a package-level regexp at line ~1223 (Laravel 9 validateUrl, RE2, linear time). Probe results: "https://example.com/feed.png", "http://localhost:8080/x?y=1#z", "https://przykład.pl/ścieżka", "ftp://a.b" match; "not a url", "example.com", "javascript:alert(1)", " https://example.com", "https://example.com " do not. +- Values reaching lagoon.Validate from cabana come from model struct fields (`valuesForRules`, modules/cabana/crud.go:1449): types like string, *string (nil or pointing to ""), named string types. `isEmptyValue` treats a non-nil pointer to "" as NOT empty, so the new checks must dereference and treat a blank string themselves. +- validateMessage (424-472): key is "lagoon::validate." + rule, except "between.numeric" -> "lagoon::validation.between.numeric"; when the translator is nil or has no line, an English fallback switch answers. cabana passes a nil translator, so the fallbacks are what admin users see. +- Catalogs: modules/phrasebook/lang/{en,pl}/validate.yaml are flat (required, email, between, ... mimes) and have no string/url keys. modules/phrasebook/lang/{en,pl}/validation.yaml (the request catalog) already have max.string ("The :attribute may not be greater than :max characters." / ":attribute nie może być dłuższy niż :max znaków."), min.string ("The :attribute must be at least :min characters." / ":attribute musi mieć przynajmniej :min znaków."), en url "The :attribute format is invalid.", pl url "Format :attribute jest nieprawidłowy.". Do NOT edit validation.yaml (pl `string` there is still English; out of scope, report it). +- Test helpers in package lagoon (validate_request_test.go): `requestTranslator(t)` loads both en/pl validate.yaml and validation.yaml from modules/phrasebook/lang; `inLocale("pl")` gives a context with that locale. Example of a translated assertion: TestValidateNumericRangeMessageTranslated in validate_test.go. +- cabana settings test patterns: modules/cabana/settings_secret_test.go (a settings model type, `secretSettingsItem` building a pact.SettingsItem with NewModel and an fstest.MapFS fields.yaml, `compileSetting("acme.unit", item, fsys)`, and the DB-free sequence `lagoon.Fill(model, settingWritable(setting), projectSettingBody(setting, body), false)`); modules/cabana/metadata_settings_test.go TestSettingsSingletonLifecycle (Postgres: `_, db := newListService(t)` which skips under -short, DropTable + AutoMigrate the model, `SettingsService{DB: db}.Put(ctx, setting, body)`, `assertSettingCount`-style row count). A failed Put returns `*ValidationError` whose `Details` is `map[string]any` holding `[]string` per field (`validationDetails`). +- Baseline at planning time (2026-11-04): `go test -short -count=1 ./modules/lagoon -run TestValidate`, `go test -short -count=1 ./modules/cabana -run 'TestSettingsSecret|TestSettingsSingletonLifecycle'` and `go test -count=1 ./cmd/summer -run TestDocsTree` all pass; Docker 29.9.1 is reachable from this shell (if the socket is denied, run the Docker tests through `sudo -u golem` per CLAUDE.md). modules/ was clean. +- Several sessions work in this repo: run `git status --short` before editing and before staging; stage only the files listed in each task by path; leave other uncommitted edits alone. Commit messages carry no co-author trailer (user rule). + + + + + + Task 1: string and url tokens in lagoon.Validate, proven through a cabana settings save + /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate.go, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate_test.go, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/phrasebook/lang/en/validate.yaml, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/phrasebook/lang/pl/validate.yaml, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/cabana/settings_string_url_test.go, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/README.md, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/docs/database/casts-and-validation.md + modules/lagoon/validate.go (whole file), modules/lagoon/validate_rules.go lines 285-310 and 1215-1225, modules/lagoon/validate_test.go, modules/lagoon/validate_request_test.go lines 20-50, modules/cabana/settings_secret_test.go lines 1-60 and 209-230, modules/cabana/metadata_settings_test.go lines 40-60 and 109-160, modules/cabana/settings.go lines 200-275 and 330-407, modules/phrasebook/lang/en/validate.yaml, modules/phrasebook/lang/pl/validate.yaml, modules/lagoon/README.md line 32, docs/database/casts-and-validation.md line 111 + + - nullable|string|max:255 on field title: "hello", nil, "", " ", (*string)(nil), pointer to "", pointer to "x", a named string type value all pass; int 5, bool true, []string{"a"} fail with exactly ["The title must be a string."]. + - string without nullable: "" and nil pass (blank values skip string); 5 fails. + - required|string on "" gives exactly ["The title field is required."] (no string message). + - nullable|url|max:500 on field website: "https://example.com/feed.png", "http://localhost:8080/x?y=1#z", "https://przykład.pl/ścieżka", nil, "", " ", pointer to "" pass; "not a url", "example.com", "javascript:alert(1)", pointer to "nope", int 42 fail with exactly ["The website format is invalid."]. + - Translated (requestTranslator + inLocale("pl")): string failure on title is "title musi być ciągiem znaków."; url failure on website is "Format website jest nieprawidłowy.". + - TestValidateUnrecognizedRule and every other existing test in validate_test.go pass unchanged. + - cabana TestSettingsStringURLRules (no Docker): a compiled settings model with title *string (nullable|string|max:255), website *string (nullable|url|max:500), language string (nullable|string|max:10), run through the same sequence Put uses (lagoon.Fill with settingWritable/projectSettingBody, then lagoon.Validate with settingRules and valuesForRules, nil tx, nil translator): body {title: "Feed", website: "https://example.com/feed.png", language: "pl"} gives no errors and no error return; {website: ""} and {website: nil, title: nil} give no errors; {website: "not a url"} gives {"website": ["The website format is invalid."]}. + - cabana TestSettingsStringURLRulesPostgres (skips under -short via newListService): SettingsService.Put with the valid body succeeds (Exists true, row count 1); Put with {website: "not a url"} returns a *ValidationError whose Details["website"] is []string{"The website format is invalid."}, and the stored row still holds the earlier website. + + +Write the tests first, run them red (the string/url tokens currently return the unrecognized-rule error), then implement. Commit once at the end of this task. + +Tests in modules/lagoon/validate_test.go: add TestValidateStringRule, TestValidateURLRule and TestValidateStringURLTranslated as table tests covering every lagoon behavior row above (Validate with nil tx, nil model, nil translator unless stated; for pointer cases build local string variables and pass their addresses; for the named type declare a local type over string inside the test). Assert exact message slices, not just non-empty. + +New file modules/cabana/settings_string_url_test.go (package cabana, internal test like settings_secret_test.go): declare a settings model type stringURLSettings with ID uint primary key, Title *string column title, Website *string column website, Language string column language; TableName cabana_string_url_settings; Fillable title, website, language; Rules() returning the three rule strings from the behavior list (the Journal shapes). Build the pact.SettingsItem with NewModel and an fstest.MapFS models/settings/fields.yaml declaring the three fields as type text, compile it with compileSetting, then write TestSettingsStringURLRules (DB-free sequence, table of bodies) and TestSettingsStringURLRulesPostgres (newListService, DropTable then AutoMigrate the model, SettingsService{DB: db}.Put, errors.As to *ValidationError, read the row back with db.First). Name helpers so they do not collide with existing cabana test helpers (grep before naming). + +Implementation in modules/lagoon/validate.go: +- In validateField add locals hasString and hasURL; add `case "string":` and `case "url":` to the token switch that only set those flags (no immediate check, so the nullable short-circuit and the required check keep their order). Do not change any other case or the default branch. +- Add an unexported helper ruleString(val any) (string, bool): dereference pointers and interfaces (a nil one gives "", false), return the reflect String() and true when the Kind is reflect.String (named string types included), otherwise "", false. +- Insert a new block right after the `if required && isEmptyValue(val)` return and before `usedBetween :=`, entered only when hasString or hasURL: compute s, isStr from ruleString; the value is blank when isEmptyValue(val) is true or isStr and strings.TrimSpace(s) is empty; a blank value skips this block entirely (Laravel skips non-implicit rules for a blank string; required already handled emptiness). Otherwise: if hasString and not isStr, return only the "string" message; if hasURL and (not isStr or urlPattern does not match s), collect the "url" message (reuse urlPattern from validate_rules.go as is: no trimming, no new pattern). Return the collected messages when there are any; otherwise fall through to the existing tags loop and unique check untouched. +- validateMessage: add fallback cases "string" -> "The " + attr + " must be a string." and "url" -> "The " + attr + " format is invalid." (the en lines of the request catalog). +- Update the Validate doc comment only if it lists tokens (it does not today; leave it otherwise). + +Catalogs: append to modules/phrasebook/lang/en/validate.yaml the keys string ("The :attribute must be a string.") and url ("The :attribute format is invalid."); append to modules/phrasebook/lang/pl/validate.yaml string (":attribute musi być ciągiem znaków.") and url ("Format :attribute jest nieprawidłowy."), keeping the files' quoting style. Do not touch validation.yaml. + +Docs, same commit (CLAUDE.md documentation rule): in modules/lagoon/README.md line 32 add `string` and `url` to the lagoon.Validate rule list and one sentence: `string` requires a string value (a pointer to one included) and `url` a URL matching Laravel's pattern, the one lagoon.ValidateRequest uses; both skip an empty or blank value, which is left to `required`. In docs/database/casts-and-validation.md line 111 add the two rules to "The supported rules are ..." and the same empty-value sentence in plain prose. Do not rewrite the min/max length sentence in this task (Task 2 owns it). + +Run `git status --short`, stage exactly the seven files of this task by path, and commit as: feat(lagoon): accept string and url rules in lagoon.Validate (quick-261104-f4x). No co-author trailer. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./modules/lagoon ./modules/cabana && go test -short -count=1 ./modules/lagoon -run 'TestValidate' && go test -short -count=1 ./modules/cabana -run 'TestSettingsStringURLRules|TestSettingsSecret' && go test -count=1 ./modules/cabana -run 'TestSettingsStringURLRulesPostgres|TestSettingsSingletonLifecycle' && go test -count=1 ./cmd/summer -run TestDocsTree && git diff --quiet HEAD -- modules/phrasebook/lang/en/validation.yaml modules/phrasebook/lang/pl/validation.yaml + + The Journal-shaped rules validate through the cabana settings save path, with and without Postgres; string/url failures give the exact en messages and the pl lines with a translator; blank values pass; all existing lagoon/cabana tests touched by the run are green; README and the docs page list string and url; validation.yaml files are unchanged; one commit with the seven files exists. + + + + Task 2: min and max as character length on string and url fields + /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate.go, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/validate_test.go, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/cabana/settings_string_url_test.go, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/lagoon/README.md, /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/docs/database/casts-and-validation.md + modules/lagoon/validate.go (the block added in Task 1, validateMessage), modules/phrasebook/lang/en/validation.yaml max/min sections (lines 62-73), modules/phrasebook/lang/pl/validation.yaml lines 36-46, modules/cabana/settings_string_url_test.go + + - nullable|string|max:255: a 255-character ASCII value passes; 256 characters fail with exactly ["The title may not be greater than 255 characters."]; 255 letters "ż" (510 bytes) pass; 256 "ż" fail. + - string|min:3: "ab" fails with exactly ["The title must be at least 3 characters."]; "abc" passes; "" and nil pass (blank skip); required|string|min:3 on "" gives only the required message. + - nullable|url|max:30: "https://example.com/feed.png" passes; "https://example.com/a-very-long-path/feed.png" fails with exactly ["The website may not be greater than 30 characters."]; "this is not a url and it is far too long" (40 characters) fails with ["The website format is invalid.", "The website may not be greater than 30 characters."] in that order. + - string with a non-string value and max (string|max:3 on int 12345) gives only the string message. + - string|max:abc returns a non-nil error containing "max:abc" (malformed argument fails loudly, like a malformed between). + - Translated with pl: 256 characters under max:255 gives "title nie może być dłuższy niż 255 znaków."; "ab" under min:3 gives "title musi mieć przynajmniej 3 znaków.". + - Unchanged behaviour: max:5 and nullable|max:5 WITHOUT string/url on a 10-character value still give no error (assert it, so the no-behaviour-change guarantee is pinned); string|between:1,5 keeps the existing between check; numeric|min:0|max:10 and integer|between rules give the same results as before (existing tests). + - cabana TestSettingsStringURLRules gains a case: title of 256 characters gives {"title": ["The title may not be greater than 255 characters."]}; TestSettingsStringURLRulesPostgres gains the same refused Put, leaving the stored title unchanged. + + +Tests first: add TestValidateStringLength (table) and extend TestValidateStringURLTranslated in modules/lagoon/validate_test.go with every behavior row above, including the pinned no-change rows for max without string/url; extend both cabana tests with the too-long title case. Run red, then implement. Commit once at the end. + +In validateField's string/url block from Task 1, after the string and url checks and only when the field is not numeric or integer (numericRange false) and the dereferenced value is a non-blank string: count characters with utf8.RuneCountInString (Laravel's mb_strlen; add the unicode/utf8 import). When minArg is set, parse it with strconv.Atoi after trimming; when maxArg is set, likewise; a parse failure returns nil and fmt.Errorf("lagoon: unrecognized validation rule %q", "min:"+minArg) or the max equivalent. A count below min appends the "min.string" message, above max the "max.string" message, both with params min and max set from minArg and maxArg. The messages collected in the block keep the order string, url, min/max. The string-type failure still returns alone. Leave `between` exactly as it is (its existing check in the tags block), and leave min/max on fields without string/url untouched: the tags loop keeps skipping them. + +validateMessage: route "min.string" and "max.string" to the keys lagoon::validation.min.string and lagoon::validation.max.string the same way "between.numeric" is routed (one condition covering the three), and add English fallback cases "min.string" -> "The " + attr + " must be at least " + params["min"] + " characters." and "max.string" -> "The " + attr + " may not be greater than " + params["max"] + " characters.". + +Docs, same commit: in modules/lagoon/README.md line 32 add that on a `string` or `url` field `min` and `max` compare the length in characters with the `lagoon::validation` string size messages. In docs/database/casts-and-validation.md line 111 replace "On a field that is `integer` or `numeric`, `min`, `max` and `between` compare the number; on other fields they compare the length." with an accurate statement: on an `integer` or `numeric` field `min`, `max` and `between` compare the number; on a `string` or `url` field `min` and `max` compare the length in characters (`The title may not be greater than 255 characters.`); `between` compares the length on any other field. Keep the rest of the paragraph. + +Final gate before committing: go vet ./... and go test -short ./... stay green (report any failure that is pre-existing and unrelated rather than fixing it). Run `git status --short`, stage exactly the five files of this task by path, and commit as: feat(lagoon): check min and max as character length on string and url fields (quick-261104-f4x). No co-author trailer. No tag, no push. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test -short -count=1 ./modules/lagoon ./modules/cabana ./modules/phrasebook && go test -count=1 ./modules/cabana -run 'TestSettingsStringURLRules|TestSettingsSingletonLifecycle' && go test -count=1 ./modules/lagoon -run 'TestValidate' && go test -count=1 ./cmd/summer -run TestDocsTree && go test -short -count=1 ./... + + string|max:255 and url|max:500 enforce character length with Laravel's string size messages in en and pl; malformed min/max arguments on string/url fields fail loudly; rule strings without string or url behave exactly as before (pinned by a test); between is unchanged; README and docs describe the length behaviour accurately; vet and the short suite are green; one commit with the five files exists. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| admin client -> cabana settings Put | Settings form values (title, URL, free text) are client-controlled strings validated by lagoon.Validate before they are stored and later rendered by plugins (for example an RSS image URL) | +| plugin author -> model Rules() | Rule strings are code, compiled into the binary; a malformed argument must fail loudly rather than silently accept input | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-261104f4x-01 | Tampering | validateField url case | medium | mitigate | url reuses Laravel's urlPattern (scheme allowlist plus ://), so javascript:, schemeless and whitespace-padded values are refused; tests pin "javascript:alert(1)" and padded URLs as failures. Output escaping stays the rendering plugin's job | +| T-261104f4x-02 | Denial of service | urlPattern match and rune count | low | mitigate | RE2 regexp and utf8.RuneCountInString are linear in input length; request bodies are already bounded by the server's body limits; no backtracking pattern is added | +| T-261104f4x-03 | Tampering | min/max arguments on string/url fields | low | mitigate | A non-integer min/max argument returns the unrecognized-rule error instead of being treated as 0 or skipped, so a typo in Rules() cannot disable a length limit unnoticed | +| T-261104f4x-04 | Information disclosure | validation messages | low | accept | Messages name only the attribute and the rule bound, never the submitted value | +| T-261104f4x-SC | Tampering | go module installs | low | accept | No new dependency; standard library (unicode/utf8, strconv, reflect) and the existing urlPattern only | + + + +- No Docker: go test -short -count=1 ./modules/lagoon ./modules/cabana ./modules/phrasebook and go test -short ./... pass; go vet ./... is clean. +- Docker (testcontainers Postgres): TestSettingsStringURLRulesPostgres, TestSettingsSingletonLifecycle and the lagoon TestValidate* tests pass (through sudo -u golem if the Docker socket is denied; the summary says which ran). +- Docs: go test -count=1 ./cmd/summer -run TestDocsTree passes. +- The two task commits (git show --stat on each) touch only the seven listed files (other sessions may commit in between; check by commit, not by HEAD~2); modules/phrasebook/lang/*/validation.yaml and every sibling repo are untouched. + + + +- Saving Settings > Journal (rules nullable|string|max:255, nullable|url|max:500) no longer fails with the unrecognized-rule error; invalid URLs and over-long values get Laravel-shaped messages. +- Existing rule strings give the same results as before. +- Two commits on master in summercms.go (tokens; length), each with its tests and doc updates, no co-author trailer, no tag or push. +- The summary reports two follow-ups for the user to decide: (1) min/max on fields without string/url (for example the README example required|max:255|unique:posts, or nullable|max:255) are still not enforced although the old docs claimed they compared the length; (2) modules/phrasebook/lang/pl/validation.yaml still has an English `string` line. + + + +Create `/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-SUMMARY.md` when done + diff --git a/.planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-SUMMARY.md b/.planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-SUMMARY.md new file mode 100644 index 00000000..c064dcec --- /dev/null +++ b/.planning/quick/261104-f4x-lagoon-validate-string-and-url-rules/261104-f4x-SUMMARY.md @@ -0,0 +1,102 @@ +--- +phase: quick-261104-f4x +plan: 01 +subsystem: lagoon +tags: [validation, lagoon, cabana, settings, phrasebook] +status: complete +requires: [] +provides: + - "lagoon.Validate accepts the Laravel rule tokens string and url" + - "min/max as character length on string and url fields" +affects: + - modules/cabana settings saves (SettingsService.Put) and controller saves that use lagoon.Validate +tech-stack: + added: [] + patterns: + - "reuse of validate_rules.go urlPattern (Laravel 9 validateUrl) in the model validator" +key-files: + created: + - modules/cabana/settings_string_url_test.go + modified: + - modules/lagoon/validate.go + - modules/lagoon/validate_test.go + - modules/phrasebook/lang/en/validate.yaml + - modules/phrasebook/lang/pl/validate.yaml + - modules/lagoon/README.md + - docs/database/casts-and-validation.md +decisions: + - "string and url skip an empty or blank value (nil, nil pointer, pointer to empty string, whitespace-only); required stays the rule that rejects emptiness" + - "On a string or url field, a min/max argument that is not a whole number is the unrecognized-rule error whatever the value (checked right after the token loop, not only on non-blank values)" + - "min/max on fields without string/url stay unenforced (no behaviour change for existing rule strings); left to the user as a follow-up" +metrics: + duration: ~25min + completed: 2026-11-04 +estimate: + tokens: 45000 + tasks: 2 +actuals: + tokens: 7111 + tasks: 2 + commits: 2 +plan_head_before: 1e38cb2 +plan_head_after: d166f1c +--- + +# Quick 261104-f4x: lagoon.Validate string and url rules Summary + +`lagoon.Validate` now accepts Laravel's `string` and `url` rule tokens. `url` uses the same Laravel 9 `urlPattern` as `lagoon.ValidateRequest`. On a string or url field, `min` and `max` compare the length in characters (runes). Settings > Journal saves (`nullable|string|max:255`, `nullable|url|max:500`) no longer fail with `lagoon: unrecognized validation rule "string"`. + +## Commits + +| Task | Commit | Message | +|------|--------|---------| +| 1 | d2ce3a2 | feat(lagoon): accept string and url rules in lagoon.Validate (quick-261104-f4x) | +| 2 | d166f1c | feat(lagoon): check min and max as character length on string and url fields (quick-261104-f4x) | + +`git rev-list --count 1e38cb2..HEAD` is 3. The extra commit is 617eaef (`docs: capture todo - Enforce min/max length on text fields in lagoon.Validate`), made by another session between the two task commits. It is not part of this plan, so `commits: 2` counts only the commits tagged `quick-261104-f4x` in that range. Each task commit touches only its listed files (checked with `git show --stat`). There are no co-author trailers, no tag and no push. + +## What changed + +- `validateField` has new `string` and `url` cases. They only set flags, so the nullable short-circuit and the required check keep their order. The new check runs after the required check. A blank value skips it. A value that is not a string fails `string` alone. `url` fails a non-string or a value that does not match `urlPattern` (no trimming, so `" https://example.com"` fails). +- The new `ruleString` helper dereferences pointers and interfaces and accepts named string types. +- Length check (Task 2): when the rules contain `string` or `url` but not `numeric` or `integer`, min and max are parsed with `lengthBound`. The length is `utf8.RuneCountInString`. The messages come in the order string, url, min/max, and use `lagoon::validation.min.string` and `lagoon::validation.max.string`, with English fallbacks for a nil translator (which is what cabana passes). +- New catalog keys `lagoon::validate.string` and `lagoon::validate.url` in en and pl. `validation.yaml` is unchanged. +- `modules/lagoon/README.md` and `docs/database/casts-and-validation.md` list `string` and `url`, explain how blank values are handled, and describe the length behaviour accurately. + +## Test results + +- `go vet ./...` clean at both commits. +- `go test -short -count=1 ./...` green at both commits. +- `go test -count=1 ./modules/lagoon/... ./modules/cabana/...` with Docker passed (lagoon 21s, lagoon/attach 50s, cabana 97s). Docker was reachable directly; `sudo -u golem` was not needed. +- `TestSettingsStringURLRulesPostgres` and `TestSettingsSingletonLifecycle` ran against testcontainers Postgres and passed (they were not skipped). +- `go test -count=1 ./cmd/summer -run TestDocsTree` passed. +- New tests: `TestValidateStringRule`, `TestValidateURLRule`, `TestValidateStringURLTranslated` (pl), `TestValidateStringLength` (lagoon); `TestSettingsStringURLRules` and `TestSettingsStringURLRulesPostgres` (cabana). In each task the tests failed first with the unrecognized-rule error or the missing length message. + +## Deviations from Plan + +**1. [Rule 2 - Correctness] A malformed min/max bound fails whatever the value** +- **Found during:** Task 2 +- **Issue:** The plan parses min/max inside the block that runs only for non-blank string values. A typo like `string|max:abc` would then go unnoticed while the field is empty or null, which is weaker than threat T-261104f4x-03 asks for. +- **Fix:** `lengthBound` runs right after the token loop for every rule string that contains `string` or `url` without `numeric`/`integer`. A negative bound is refused too. Only rule strings that errored before this change are affected. The test pins the error for both `"x"` and `nil`. +- **Files modified:** modules/lagoon/validate.go, modules/lagoon/validate_test.go +- **Commit:** d166f1c + +There were no other deviations. The tracer gate after Task 1 re-ran the verify chain end to end and it passed. + +## Follow-ups (user decision) + +1. **min/max are not enforced on non-numeric fields that have no `string`/`url`.** For example, `required|max:255|unique:posts` or `nullable|max:5` with a 10-character value still passes. The old docs line said those rules "compare the length", which was wrong; the docs now describe the actual behaviour. Changing this changes results for existing rule strings, so the user should decide. Another session has already captured it as a todo: `.planning/todos/pending/2026-11-04-lagoon-validate-min-max-length-on-text-fields.md` (commit 617eaef). +2. **The `string` line in `modules/phrasebook/lang/pl/validation.yaml` (the request catalog) is still English.** It was left alone because it is out of scope. The new `lagoon::validate.string` key in `pl/validate.yaml` is in Polish. + +## Known Stubs + +None. + +## Threat Flags + +None. No new endpoints or trust-boundary surface: the change only tightens validation on an existing save path. + +## Self-Check: PASSED + +- FOUND: modules/cabana/settings_string_url_test.go, modules/lagoon/validate.go, modules/phrasebook/lang/{en,pl}/validate.yaml +- FOUND: d2ce3a2, d166f1c in git log