@W-23563851: extend CAP security scanner with S17/S18/S19 (WARN)

[jbisaSF]

Summary

Extends .github/scripts/security-scan.sh with three new WARN-tier rules surfaced by the Avalara Tax v1.1.0 CAP review:

• S17 — BM controller guard.ensure([...]) includes a state-changing method (post/put/patch/delete) but omits 'csrf'. Scoped to /bm_cartridges/ so storefront controllers with their own CSRF-token flow are not flagged.
• S18 — Logger.(warn|error|info|debug|trace)(... JSON.stringify(err|error|e|response|svcResponse|svcResult|result ...)) — raw error/response objects stringified into logs. Log only error.message or a redacted status object.
• S19 — encodeURI(...) with a + concatenation or template-literal interpolation in the argument. encodeURI preserves URL delimiters (/ ? & = #) and single quotes; use encodeURIComponent per user-provided segment/param.

All three are WARN, not BLOCK, so exit code and total blocking-findings count are unchanged on already-shipped apps. Comments are filtered via the existing strip_comments helper.

Files changed

• .github/scripts/security-scan.sh — S17/S18/S19 blocks added after S16, using the existing warn / strip_comments / head -N helper pattern.
• .github/scripts/test-security-scan.sh — fixture-based positive and negative cases per rule.
• .claude/skills/shared/security-rules.md — canonical rule doc (which .claude/skills/validate-app/references/security-scan.md links to) updated with a one-paragraph description and suggested fix per new rule.

Acceptance criteria — status

• AC1 — Warnings emitted per rule. S18 verified on Avalara v1.1.1 (3 warnings on logger.error(... JSON.stringify(response.details))). S17/S19 covered by fixture tests only — v1.1.0 (the reviewed pre-fix version) is no longer in the local commerce-apps-testing registry (removed in a60c0f4), and v1.1.1+ have no BM cartridge JS or encodeURI misuse. Shape correctness is proven by the fixtures.
• AC2 — All three rules WARN, exit 0. Avalara blocking count unchanged (0 → 0).
• AC3 — Scoping guards in place. S17: file-path check for /bm_cartridges/. S18: identifier list matches the WI verbatim. S19: requires + or \${ inside encodeURI(...); \bencodeURI\b does not match encodeURIComponent(.
• AC4 — All three rules pipe through strip_comments. Negative fixtures for commented forms.
• AC5 — S17/S18/S19 documented in .claude/skills/shared/security-rules.md with description and suggested fix.
• AC6 — Byte-identical scanner output on commerce-noibu-analytics-app-v0.1.0 and commerce-vertex-tax-app-v1.0.0 vs. main.

Test plan

• bash .github/scripts/test-security-scan.sh — 123 passed, 0 failed (was 100 before this change).
• bash .github/scripts/security-scan.sh <avalara-v1.1.1> — 3 S18 warnings, 0 S17, 0 S19, 0 blocking findings, exit 0.
• bash .github/scripts/security-scan.sh <noibu-analytics-v0.1.0> and <vertex-tax-v1.0.0> — byte-identical output vs. main.
• CI (GitHub Actions security.yml, test-scripts.yml, verify-zip.yml) — will report after the PR build completes.

Notes

• The WI (Details__c) references commerce-apps-testing/.github/scripts/security-scan.sh as the file to touch; the scanner is duplicated verbatim in commerce-apps and commerce-apps-testing, and this PR lands the fix in commerce-apps at the user's direction.
• AC1 as literally written cites v1.1.0, which was removed from the registry before this WI was scheduled; the empirical S18 firing on v1.1.1 (which carries the same shape forward) plus the fixture harness is the substitute demonstration.

🤖 Generated with Claude Code