`verification/TK-11350/engineering-review.md`

55 lines

# TK-11350 engineering review of TK-11341

Status: complete for scoped read-only source/regression review; NOT a fleet launch certification. Owner integration_review; delegation root -> integration_review, depth 1. No source edits, commits, deployments, or account mutations. Parent owns acceptance and full-domain live scan.

## Critical / must fix

### P1 — AdSense is blocked by the sites' own CSP on three live deployments

The injected loader is incompatible with the existing script-src. Live HTTPS response headers independently confirm this on interiordesignershowroom.com, nationalpaperhangers.com, and starsofdesign.com (all returned 200). Each excludes pagead2.googlesyndication.com, so browser execution is forbidden even though source-text verification passes.

Exact anchors: /Users/macstudio3/Projects/interiordesignershowroom/lib/render.js:266 (new loader), server.js:35 and :40 (CSP); /Users/macstudio3/Projects/nationalpaperhangers/views/partials/head.ejs:4 (new loader), server.js:52 (script list); /Users/macstudio3/Projects/starsofdesign/views/partials/head.ejs:4 (new loader), server.js:36 (script list), :40 and :41 (connect/frame restrictions).

Fix: add the specifically required AdSense origins/nonces to each applicable script/frame/connect policy while retaining the security policy, then test actual browser script loading and request/iframe boundaries. Do not just disable CSP. Header evidence: /tmp/TK-11350-{interiordesignershowroom.com,nationalpaperhangers.com,starsofdesign.com}.headers.

### P1 — Bulk injection now executes ad code inside private messaging and curator/admin surfaces

/Users/macstudio3/Projects/bubbesblock/public/inbox.html:3 loads AdSense before the UI fetches private conversations at :24 and :30. Live https://bubbesblock.com/inbox returned 200 with the loader. /Users/macstudio3/Projects/WallpapersBack/wallpapersback-fork/public/luxe-curator.html:4 also loads it, confirmed live at https://wallpapersback.com/luxe-curator.html (200). Other modified operational files include seam-fix-curator.html and subject-mismatch.html. This adds third-party execution inside operational/user-private journeys, outside ordinary content monetization.

The iwascute root layout also includes it at /Users/macstudio3/Projects/iwascute/src/app/layout.tsx:33, enclosing /admin, /dashboard, /licensor, /login. This part is SOURCE ONLY: live /admin HTML had no loader mention, so local/current deployment parity is unproven.

Fix: scope ad rendering to explicitly approved public content routes/templates. Remove from inbox, notifications, curator and authenticated account/admin journeys; for Next use a public route-group layout. Verify authenticated and unauthenticated negative cases have zero ad requests, not merely zero visible ads. Evidence: /tmp/TK-11350-bb-inbox.html and /tmp/TK-11350-wpb-curator.html.

## Warnings / should fix

### P2 — Six inline repos have duplicate loaders because injection also matched <header>

At /Users/macstudio3/Projects/philipperomano/server.js:435 the loader is correctly in head; :594 repeats it inside header. The a99dece commit shows both additions. Live https://philipperomano.com/ returns TWO executable loader script tags. Also present in carmelwallpapers/server.js:281,:408; customdigitalmurals/server.js:346,:497; naturaltextilewallpaper/server.js:326,:525; designerwallcoverings/server.js:266,:399; hollywood-wallcoverings/server.js:252,:629,:671 (two alternate header templates, not necessarily three per page).

Fix: delete header copies and make injector match an actual head element with a word/tag boundary rather than the head prefix; assert exactly one executable loader per representative rendered route. Evidence /tmp/TK-11350-pr.html. Other five source cases not separately fetched; designerwallcoverings flagged as undeployed in rollout ticket.

### P2 — HomesOnSpec enables invalid manual ad units when enabling Auto Ads

The changed default at /Users/macstudio3/Projects/homesonspec/apps/web/src/components/AdSlot.tsx:19 turns CLIENT on even with no environment settings. Existing guards at :51 and :59 then render and push both manual units without checking slot. Layout.tsx:80 and :85 receive optional slot env values. Live HTTPS homepage returns TWO adsbygoogle ins elements with publisher ID but NO data-ad-slot. These cannot identify their intended manual units; the empty Advertisement zones now render. Source comment claiming no env means no ads is stale.

Fix: keep the single Auto Ads loader independently enabled, but require a valid configured slot before both rendering a manual unit and pushing its queue entry (e.g. if (!CLIENT || !slot) return null, matching effect guard). If manual units are intended, configure actual slot IDs before deployment. Evidence /tmp/TK-11350-hos.html. No ad request errors were claimed; only server-rendered missing attributes verified.

### P2 — Deployment success counters do not measure successful deployments

/Users/macstudio3/d.sh:44 ends reload/restart with `|| true`, discarding process failures. :45 probes HTTP localhost without following HTTPS and :46 increments success solely after rsync succeeds, even when HTTP is 502, 401, 301, or empty. Thus the ticket's 78/78 and 0 failed do not certify operational health. The stale runbook /Users/macstudio3/.claude/yolo-queue/pending-approval/TK-11341-adsense-rollout-deploy.md:108-109 checks source string presence and truncates duplicate results with head -1. It cannot detect any of the defects above or establish rendered ads.

Fix: require successful restart, healthy final HTTPS page after bounded retries, correct public ads.txt status/body/content type, exactly one actual browser loader and no CSP error, and expected authenticated/negative routes. Fail with nonzero exit for failed targets; do not label a plain redirect success as backend health. Existing server backups remain valuable.

## Suggestions

Replace global textual insertion with small explicit integration types and a public-route allowlist. Persist a per-domain acceptance manifest: commit, runtime route, expected ad mode (auto/manual), loader count, ads.txt result, CSP/browser result, and exclusion-path assertions. Store checks alongside the rollout so future re-runs detect the same regressions.

## Checks and limits

- Read canonical TK-11341 and its deploy runbook. Enumerated all 81 cited commit file lists into /tmp/TK-11350-commits.json; inspected representative inline, static, EJS, and Next diffs.
- Read-only source scan of modified paths identified all six duplicate inline implementations and operational-page placements.
- Read-only source CSP scan across the rollout repos plus live HTTPS header confirmation for all three blocking policies.
- Read-only live HTTPS representative captures for Phillipe Romano, HomesOnSpec, WPB curator, BubbesBlock inbox, and iwascute admin; all 200. No cookies/credentials or writes used. Initial sandbox DNS failures were retried via approved curl escalation and succeeded.
- Browser rendering/actual ad fill, consent flow, account approval, authenticated pages and responsive placement were not exercised by this engineering child. Parent/other reviewer owns policy and browser checks. No claim of secrets exposure, ad impression/revenue, or actual private-data transmission is made.
- starsofdesign is currently HTTP 200, so the historical 502 is not a current outage finding; the current confirmed problem there is CSP blocking.