feat(deploy): Procfile + composer stub so the production Heroku app boots the rebuilt SPA - #164
Conversation
Root Procfile boots heroku-php-apache2 at the default docroot (app root); the committed .htaccess rewrites non-file requests to index.php, which serves app/dist and applies the legacy 301 map. composer.json pins the PHP runtime to ~8.3.0 and is required for PHP buildpack detection; matching platform-only composer.lock generated with composer update --no-install --ignore-platform-reqs. docs/deploy.md is the transition runbook (buildpack detection, cutover checklist after PR #155 merges, immutable-release rollback, DNS/SSL untouched at Cloudflare). Handoff section in docs/verification-report.md now points at it. Shim verified: php -l clean on 8.4, full verify-shim.sh redirect table green locally.
![obvious-autobuild-staging[bot]](https://avatars.githubusercontent.com/u/31830338?s=60&v=4){kind=small}
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT (1 Medium finding)
Summary
- Blocker: 0
- High: 0
- Medium: 1
Deploy config verified against the release tree at merge-base 2031376: Procfile (clean LF, correct buildpack invocation, default docroot = app root where index.php and .htaccess live), platform-only composer.lock pinning ~8.3.0 (matches the CI lint runtime), and the cutover checklist's redirect table matches .github/scripts/verify-shim.sh row for row. package.json lives under app/, so there is no root-level Node/PHP buildpack detection conflict. No secrets in the newly served tree.
One gap: the default docroot exposes every committed real file that isn't in the .htaccess deny list — see the inline comment on docs/deploy.md.
| `heroku-php-apache2` at the repo root. Heroku's Apache build honors the | ||
| committed `.htaccess`, so the shim behaves exactly as verified locally: | ||
|
|
||
| 1. Real files under the docroot (`img/`, `robots.txt`, `sitemap.xml`) are |
There was a problem hiding this comment.
Medium · security — Default docroot serves every committed real file, not just the three listed here. The runbook says real files under the docroot (img/, robots.txt, sitemap.xml) are served directly, but with the Procfile deliberately using the default docroot (the repo root), Apache serves any committed real file not in the .htaccess deny list — which only covers app/, .git* (also .github), and README.md. After cutover these become publicly reachable on csvjson.com: docs/verification-report.md and docs/deploy.md (internal handoff/runbook), verification-screenshots/*.png (12 internal screenshots), ISSUE_TEMPLATE.md, csvjson.sublime-project/-workspace, and .user.ini (served as text/plain if requested directly). A sensitivity scan found no credentials in them (the S3 permalink URL is already public in the client bundle), so this is internal-doc disclosure rather than a secret leak — hence Medium. The legacy CodeIgniter app never exposed these paths. Fix: extend the deny list before cutover, e.g. RewriteRule ^(docs|verification-screenshots)/ - [F,L] plus rules for ISSUE_TEMPLATE.md / .user.ini / the sublime files, update lines 29–31 so the runbook states the deny list is the only gate, and add a row to verify-shim.sh so CI pins the behavior.
There was a problem hiding this comment.
Fixed in 87ea979. The default docroot does expose every committed real file, so the deny list is now the only gate and is explicit about it:
.htaccessadditionally 403sdocs/,verification-screenshots/, all dotfiles (^\.— covers.user.ini,.gitignore,.github),composer.json/composer.lock,Procfile,ISSUE_TEMPLATE.md, and the sublime project files.index.phpmirrors the same list so the dev server matches production (it previously handed real docroot files to the built-in server raw), andverify-shim.shnow pins 404s for all of them, so CI enforces the behavior on every push.docs/deploy.mdnow says the deny list is the only gate and lists exactly what stays public (img/,robots.txt,sitemap.xml,license.txt,index.php), plus a standing note that any future root-level commit must be public by design or added to both deny lists.
All checks green locally including the 8 new internal-file rows; the public surface (img/, robots.txt, sitemap.xml, license.txt) verified still serving.
With the default docroot = repo root, Apache serves every committed real file unless denied. Extend the .htaccess deny list to repo docs, verification screenshots, dotfiles, build/dep metadata, and editor project files (403), mirror the same list in the front controller so the dev server 404s them identically, pin both behaviors in verify-shim.sh, and state in the runbook that the deny list is the only gate on docroot files.
|
Fixed the Medium finding in 87ea979: with the default docroot = repo root, the |
There was a problem hiding this comment.
Obvious Code Review — Pass 2 (head 87ea979)
Verdict: COMMENT — No findings, clean pass.
The new commit resolves the Pass 1 Medium (docroot exposure) completely:
.htaccessdeny rules fordocs/,verification-screenshots/, root dotfiles,composer.json|lock,Procfile,ISSUE_TEMPLATE.md,csvjson.sublime-*— verified with a full root-entry audit at the new head: every entry is now either public-by-design or denied, no gaps, no public path regressed.index.phpmirrors the list as 404s, correctly placed before the dev-server static handoff; regexes behavior-equivalent to the Apache rules.verify-shim.shpins 8 new 404 rows in CI.docs/deploy.mdnow states the deny list is the only gate and enumerates the public set accurately, plus a forward-looking docroot-gate note for future root commits.
|
Thanks — Pass 2 clean, and the full root-entry audit confirms no gaps and no public regressions. PR is ready for David's merge call alongside release PR #155. |
Post-cutover SEO monitoring section: Search Console baseline export is a blocking pre-cutover artifact (step 0, the only before/after evidence); week-2/week-4 checkpoints against the baseline with the specific check that legacy tool URLs show as 'Page is redirected' (not errors) in coverage; sustained >20% drop on core-tool queries at week 4 triggers a revisit, with dedicated landing pages as the recovery play — undoing the redirects is explicitly not the mitigation; satellite-tool query traffic (sql to json, data janitor) is expected to drop per the locked product decision.
There was a problem hiding this comment.
Obvious Code Review — Pass 3 (head f1a7037)
Verdict: COMMENT — No findings, clean pass.
Docs-only delta (docs/deploy.md, +41/−3). Verified:
- The new blocking step 0 (Search Console baseline export) cross-references
docs/verification-report.mdaccurately — the baseline is indeed pre-launch item 1 there. - The post-cutover monitoring section is technically sound: "Page is redirected" is the correct healthy coverage state for the 301-mapped legacy tool URLs (consistent with the redirect behavior verified in Pass 2), and the recovery play — dedicated landing pages rather than undoing the 301s — is the right call, since undoing them would resurrect retired URLs that no longer have pages.
- Canonical patch re-checked: same 8 files as Pass 2, no unexpected files.
|
Thanks — Pass 3 clean noted. Merging into release/rebuild-csvjson-spa now per the cutover plan. |
2ca3a7e
into
release/rebuild-csvjson-spa
1 participant
Why
The release branch (
release/rebuild-csvjson-spa, promotion PR #155) is verified and ready, but the production Heroku app has nothing telling it how to boot the new tree: no web process definition and no composer manifest, so the PHP buildpack would never auto-detect. Until this lands, merging #155 doesn't actually put the new site on csvjson.com.What
Minimal deploy configuration so the existing Heroku app (the one Cloudflare already proxies csvjson.com to) boots the new app on its next deploy:
Procfile— exactlyweb: heroku-php-apache2. No docroot argument: default docroot is the app root, whereindex.phplives and.htaccessrewrites every non-file request to it.composer.json+composer.lock—{"require":{"php":"~8.3.0"}}with a platform-only lock (no packages).composer.jsonis required for PHP buildpack detection; the constraint pins PHP 8.3.x so the runtime is deterministic. Lock generated withcomposer update --no-install --ignore-platform-reqs(sandbox had PHP 8.4/composer 2.8.8, so platform requirements were ignored — the lock records the~8.3.0platform requirement correctly).docs/deploy.md— the transition runbook: how the buildpack detects and boots Apache, the cutover checklist for after Release: Rebuild csvjson.com as a minimal modern converter #155 merges (301-map curl checks against production per.github/scripts/verify-shim.sh, one real legacy/<tool>/<id>permalink hydration, robots/sitemap, prerendered SEO HTML), rollback via immutable Heroku releases (heroku rollbackrestores the old app instantly), and the note that DNS/SSL stay untouched — Cloudflare already proxies csvjson.com to heroku-router.docs/verification-report.md— handoff section now points atdocs/deploy.md.Intentionally excluded: any Heroku-specific routing overrides, a docroot argument,
app.json, and CI changes — the shim lint job (php -lon 8.3) already covers PHP 8.x compatibility, and the shim itself needed no changes (verified clean withphp -lon 8.4 and the fullverify-shim.shredirect table locally).How to Review
Procfileandcomposer.jsonat repo root are the only runtime-affecting files; everything else is docs.docs/deploy.mdis the runbook to follow at cutover — the checklist there is the post-merge verification sequence.{}composer.json (would let Heroku pick latest PHP instead of pinning 8.3, the version CI lints against) and a docroot argument on the Procfile line (unnecessary — the front controller is intentionally at the web root).Verification
php -l index.php— clean (PHP 8.4 locally; CI lints on 8.3).bash .github/scripts/verify-shim.sh— all redirect-table checks green locally (test stand-ins reverted after the run; committedapp/distuntouched).composer validate— valid (warnings: no license/description, intentionally minimal).🔗 Obvious Project · 🧵 Obvious Thread