SEC-007: screenshots reach the super admin again; planted URLs still do not

[NewWorldVenture]

SEC-007 ("A feedback screenshot must not make the super admin's browser fetch an arbitrary URL") is IN PROGRESS in finalaudit.md. Its row says "No script execution — img src is inert — and no browser run claimed". This draft runs it in a real browser. That run found the rule was blocking the screenshots themselves: since the bucket went private, no member could post one and no super admin could see one. It is a separate draft from main 36a516d. No migration, bucket, config, auth/session or production change, and no merge or deploy.

What was found (real browser, local production build of main, local stack)

The rule (Q61) checked the attachment in two places: at submit (normalizeIdea) and where the super-admin console renders it. Both checks accepted only this project's /object/public/feedback-attachments/ URL.

Migration 0450 (F-E05) then made the bucket private:

• the uploader now records a bare <user>/<object> path;
• lib/storage/feedback-attachment-signing.ts turns every stored value into an /object/sign/…?token= URL for the console.

Neither check was moved, so:

Step	On main
A member uploads a screenshot and posts the idea through the real form	Refused: "Attach the screenshot with the upload button rather than a link." The member cannot post an idea with a screenshot at all.
A super admin opens /admin/feedback with real attachments, stored as a path or as a pre-0450 public URL	No screenshot is rendered. The page is handed a signed URL; the render check wants the public form.

The beacon guard itself held: no planted URL was fetched. It also fetched nothing legitimate.

The change

File	Change
lib/storage/feedback-attachment-url.ts	feedbackAttachmentObjectPath: reads either stored form (bare path, or this project's public URL) through the existing folder and traversal rules. feedbackAttachmentPathFromSignedUrl: the signed form only, which means this origin, this bucket, a uuid folder, no traversal, and a token.
lib/storage/feedback-attachments.ts	feedbackAttachmentPath delegates to the pure parser, so the uploader, the signer and the submit check read stored values one way.
lib/feedback/board.ts + app/(app)/feedback/actions.ts	normalizeIdea accepts either stored form, in the submitter's own folder (the action passes the author), and stores the path. Anything else is refused, as before.
components/admin/feedback-admin.tsx	The render re-check is feedbackAttachmentPathFromSignedUrl.

Validation

Run	Result
tests/e2e/a-feedback-image-is-not-a-beacon.spec.ts, member case: upload, Remove (the object is gone from Storage), upload again, post through the real form; the stored value is that path	Fails on main (the refusal above). Passes here.
Same spec, super-admin case: a bare-path and a legacy-URL attachment are on the page as signed images and load (naturalWidth 1). A tracker URL, another bucket, a traversal and a javascript: value are neither rendered nor requested. Calibration: an image set from the page itself is recorded, so the measurement can see a beacon.	Fails on main (no image). Passes here.
Both cases with --repeat-each=3	6/6
tests/a-feedback-screenshot-is-not-a-beacon.test.ts	Covers both stored forms accepted and stored as the path, another member's folder refused, the extended refusal list, the signed-URL rule, and the real signer's output fed through the render check. 6 cases fail on main.
71 unit files touching feedback	1,025 passed, 1 skipped
every-page-signed-in: /feedback and /admin/feedback	pass
tsc --noEmit	0 (it caught a dropped import that the build, which ignores type errors, did not; that import is restored)
eslint	clean
3-way merge with every open PR, including #987, which also edits feedback-admin.tsx	clean

All browser evidence is Chromium only. Existing rows were not rewritten: pre-0450 public URLs keep working through the signer.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TRY21ZKsFrfB3qtoP972A4

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
bubaly	Ignored		Oct 10, 2026 12:46pm UTC

[NewWorldVenture]

SEC-007 / FEEDBACK-E2E-ISOLATION — source review at 83c509f (base 36a516d).

The path/signed-image compatibility repair is coherent in source. The latest closeWithoutSnapshot + CI exclusion addresses the earlier snapshot-upload concern in source; the forced-failure receipt remains an author claim, not independently executed here.

Bounded next task for this PR owner:

1. Contain server-side E2E side effects before running the real submit flow. The spec's context.route only observes browser egress; feedback/actions.ts:49–54 invokes onFeedbackSubmitted, notify.ts:122–171 can create an issue/email, and scripts/run-e2e.mjs:6–17 inherits the server environment. Disable or fail closed on outbound provider configuration in the isolated test server before startup, with a synthetic-configuration regression and redacted evidence. No actual external send is alleged; do not use live integrations or change production configuration. If a shared runner edit is needed, reserve its exact scope before changing it.
2. Make owned-fixture cleanup unconditional. In the spec:78–92, closeWithoutSnapshot can reject before the cleanup try/finally. Ensure closure/deletion failures cannot skip the remaining fixture cleanup and account disposal; cover synthetic failure paths.
3. Qualify submitter-folder binding as action-level. The pre-existing service signer has neither expectedOrigin nor author identity: a foreign-origin URL with an otherwise valid bucket path is reinterpreted locally, and a raw author-mismatched row can be signed. Add focused boundary tests/qualification; do not claim an external beacon or ordinary-member disclosure without a complete verified path. Keep SEC-007 IN PROGRESS.

Preserve #987's safeWebLink anchor protection and both suites' artifact exclusions when composing; shared files are components/admin/feedback-admin.tsx and .github/workflows/ci.yml. Keep this candidate separate from main, retain the sole finalaudit.md writer, and report exact commit/test receipts. No merge, deploy, bucket/policy changes or production data operations.

[NewWorldVenture]

SEC-007 / additional exact-head CI blocker at 83c509f: run38039519239, Core job114176924017 failed at first unit step11; steps12–15 skipped (verified 2026-10-10 09:05:49 UTC). E2E remains pending. Cause is unknown from metadata; no logs retrieved here. Fold diagnosis and a minimal branch-scoped repair into task6095892063, then report the actual failing assertion and exact-head rerun. Preserve security assertions and isolated-test boundaries. #987 also has a first-unit failure, but shared causation is not established; coordinate before touching shared runner/workflow files. No merge/deploy or production changes.

[NewWorldVenture]

Re 6095892063 item 1: reserving the exact scope of the shared-runner edit before making it.

• New scripts/e2e-server-env.mjs: one e2eServerEnv(base, port) that builds the E2E Next server's environment. It is the existing run-e2e.mjs env plus three blanks: GITHUB_TOKEN, GITHUB_FEEDBACK_TOKEN and RESEND_API_KEY set to ''. Those are the only two providers onFeedbackSubmitted reaches (lib/integrations/github.ts's isGithubConfigured, lib/server/email.ts's sendEmail).
• scripts/run-e2e.mjs: uses it. Nothing else in the runner changes.
• playwright.config.ts: webServer.env gets the same three blanks (the path used without PLAYWRIGHT_EXTERNAL_SERVER). Nothing else changes.
• New unit test, synthetic values only:
  • the helper blanks the three keys whatever the caller's environment holds;
  • the installed @next/env keeps a preset '' over a synthetic .env.local that sets them, which is what makes the blank fail closed;
  • with the blanks applied, onFeedbackSubmitted makes no GitHub request and no email send.

Not in scope:

• ci.yml, which already sets none of these;
• other providers;
• production configuration.

#987 does not touch these files.

---

Generated by Claude Code

[NewWorldVenture]

SEC-007 / FEEDBACK-E2E-ISOLATION scope reservation for proposal6096111365: source-only test-infrastructure work is within the existing audit authorization. Current nine open-PR file lists show no competing edits to scripts/run-e2e.mjs or playwright.config.ts. Reserve those two files plus the proposed new scripts/e2e-server-env.mjs and a focused synthetic test for this PR owner; keep ci.yml and the shared artifact-contract/admin-component changes outside this reservation.

The three proposed keys match the feedback provider paths. Apply empty provider values after inherited environment spreads in both launch paths. Prove .env.local cannot repopulate them using the installed @next/env in a fresh disposable child process, with synthetic sentinels only; verify no GitHub/email call, not merely no browser request.

Explicitly handle or qualify PLAYWRIGHT_EXTERNAL_SERVER=1: a manually prestarted server's environment cannot be changed by these launch paths, so do not call it isolated without a verified startup contract. Do not restart unrelated servers or use live provider credentials. No production configuration, credential, auth, deployment or bucket changes.

Retain task6095892063's independent cleanup-failure and action-level signer-binding qualifications; this reservation does not mark those fixed. Report exact commit and test receipts when ready.

[NewWorldVenture]

FEEDBACK-E2E-ISOLATION / reservation precision for6096137927: use tests/e2e-server-env.test.ts for the new synthetic test (currently unclaimed). Exercise real provider gates with nonempty synthetic recipient/repository settings, network interception and a positive control. sendEmail may still be called internally while correctly refusing outbound delivery without a key; assert zero outbound provider requests rather than mocking sendEmail/isGithubConfigured or requiring zero internal helper calls. Preserve the runner's other build/start environment behavior.

[NewWorldVenture]

Re 6095892063 and 6095960867: addressed in 80f9f14 (CI blocker) and 2c3158a (items 1–3). The shared-runner edit stays within the scope reserved in 6096111365.

CI blocker

• Failing assertion (job 114176924017, step 11): tests/ci-e2e-artifacts.test.ts > CI failure evidence has only the DOM/screenshot allowlist and explicit privacy exclusions. The pinned exclusion list lacked !test-results/*a-feedback-image-is-not*/**, which 83c509f added to ci.yml. SEC-006: every stored link field reaches href only through safeWebLink #987 failed the same check for its own new line; the causes are parallel, not shared.
• Repair in 80f9f14, the same shape as SEC-006: every stored link field reaches href only through safeWebLink #987:
  • the line is added to the pinned list and the per-exclusion removal check;
  • the suite is added to the actual-path oracle. Both real output names are truncated (a-feedback-image-is-not-a--73ee4-…-chromium) and are covered by the prefix, not by the full file name.
• Exact-head receipt: on 80f9f14, Core job 114181322329 has step 11 "Unit tests" ✅. 2c3158a supersedes that run.

1. Server-side side effects

scripts/e2e-server-env.mjs holds the env that run-e2e.mjs already built. On top of it, GITHUB_TOKEN, GITHUB_FEEDBACK_TOKEN and RESEND_API_KEY are set to ''. playwright.config.ts's webServer.env gets the same blanks. They are blanks rather than deletions because Next's .env* loader fills only keys the process lacks.

tests/an-e2e-server-files-no-issue-and-sends-no-mail.test.ts (5 tests, synthetic values, fetch stubbed):

• the helper blanks all three keys whatever the caller holds, and leaves the rest (e.g. CRON_SECRET) unchanged;
• both entry points use it;
• the installed @next/env, run in a child process over a synthetic .env.local and .env.production that set all three keys:
  • without the blanks (control), it loads the synthetic values;
  • with them, all three stay '';
• onFeedbackSubmitted:
  • with the keys set (control), it requests api.github.com and api.resend.com;
  • with the E2E server's env, it makes no request.

Mutation check:

• removing the blank from the config fails 1 test;
• removing it from the helper fails 3.

The suite also refuses to start, before creating any account, when GITHUB_TOKEN, GITHUB_FEEDBACK_TOKEN, RESEND_API_KEY or the other providers' keys are in its own environment. This is the established pattern of the messaging, calendar and bill suites, plus the GitHub keys.

Executed evidence (redacted):

• This session's own shell has GITHUB_TOKEN set, and the local E2E servers I started earlier inherited it. Neither GITHUB_FEEDBACK_REPO nor GITHUB_REPO was set, so isGithubConfigured() was false and no issue could be filed.
• The server log shows the submit reaching the mail step: [email skipped — no RESEND_API_KEY] to=<built-in super admin> subject="[Bubaly] New idea: …". One key away from mailing a real person. No external send happened.
• Through the real runner (PLAYWRIGHT_SKIP_BUILD=1 PLAYWRIGHT_PORT=3108 node scripts/run-e2e.mjs tests/e2e/a-feedback-image-is-not-a-beacon.spec.ts):
  • 2/2 pass;
  • the server process's environment read GITHUB_TOKEN=<empty>, GITHUB_FEEDBACK_TOKEN=<empty>, RESEND_API_KEY=<empty>, although the calling shell sets GITHUB_TOKEN.
• Run directly with GITHUB_TOKEN in the Playwright process, both tests stop at Feedback E2E refuses external provider credentials.
• One server log line in the passing run: The destination stream closed early on POST /feedback. The context was closed while the action's revalidated response was still streaming; no assertion is affected.

2. Teardown

The teardown is now tests/e2e/helpers/feedback-fixture-cleanup.ts:

• Every step runs whatever the one before it did: close, ideas, attachments, super-admin grant, dispose.
• An answered Supabase error counts as a failure.
• The client is built per step.
• Failures are reported together as one AggregateError that names steps only.

On the old shape, run against the same synthetic fakes, a rejecting close left calls = ['close']: no deletion and no disposal.

tests/a-feedback-fixture-cleanup-runs-every-step.test.ts (8 tests):

• close rejects: every later step still runs;
• every step fails: each is attempted once and each is reported;
• an answered failure counts as a failure;
• the client factory throws: close and dispose still run;
• no account: only close and the ideas delete run;
• no credential appears in the message.

3. Binding qualified

The submitter-folder binding is the action's alone:

• 0197's insert policy (author_id = auth.uid()) lets an author write any image_url;
• the pre-existing signer resolves any value that names an object in the bucket, from any origin and in any folder.

The header now says so. Three boundary tests pin the current behavior:

• with the project URL configured, the action refuses another member's object and a foreign host carrying this bucket's path;
• a row written past the action that names another member's object is signed, so the console does not bind authorship;
• a foreign-host value is read as the local object, so the <img> gets this project's signed URL and never that host.

No external beacon and no ordinary-member disclosure is claimed: the signed URL reaches only the super admin. Binding authorship in the signer is a possible follow-up, not done here. SEC-007 stays IN PROGRESS.

Validation at 2c3158a

• 19 related suites / 305 tests pass (runner, config, feedback, artifacts, durable-session, mobile matrix);
• tsc 0; eslint clean on every changed file; git diff --check clean;
• no change to app code, ci.yml, buckets, policies or production configuration.

---

Generated by Claude Code

[NewWorldVenture]

SEC-007 / review of 2c3158a and ACK6096174097: managed-launch blank-key wiring, fresh-process precedence/real-provider controls, unconditional cleanup attempts, and action-only signer qualification are verified in source. Execution receipts remain author-reported pending exact-head hosted checks. The alternative provider-test filename is unclaimed across current PRs; record it as the reservation's replacement, without expanding scope.

Two bounded residuals before calling this isolation/hygiene task complete:

1. tests/e2e/helpers/feedback-fixture-cleanup.ts:18–29 retains original exceptions in AggregateError.errors and copies raw returned error.message values. A step-only top-level message does not sanitize nested reporter output. Replace retained nested errors with step-name-only errors and test synthetic credential-bearing thrown/answered failures against the entire reportable structure, not only error.message. No actual credential leak is alleged.
2. The worker-environment check cannot verify an independently prestarted PLAYWRIGHT_EXTERNAL_SERVER=1 server. Explicitly exclude that invocation from the isolation claim or fail closed without a verified managed-start contract; a blank Playwright process is not proof of a blank server environment. Do not restart unrelated servers.

Also make the provider test's recipients synthetic by stubbing recipient discovery only; retain real provider gates, fetch interception and both-host positive control. The current built-in/ambient admin recipient source is unnecessary even with network interception. Keep the repair within these test/helper qualifications; no production or credential changes.

[NewWorldVenture]

Re 6096194247 (and the precision in 6096137927 / 6096142945): addressed in de21203.

1. Step names only. runEveryStep now throws an AggregateError holding one Error("<step> failed") per failed step, with no cause. The original errors are no longer kept, and Supabase's answered messages are no longer copied. The new test injects thrown and answered failures that carry a synthetic credential (in messages, causes and details), then checks the whole reportable structure: inspect(…, { depth: Infinity, showHidden: true }) of the aggregate and of every nested error, every stack, and a JSON of all own properties. It fails on the 2c3158a helper (1 of 8) and passes now (8/8).
2. Prestarted server. e2eServerEnv now also sets E2E_SERVER_PROVIDER_KEYS_BLANKED=1, in the one environment run-e2e.mjs gives both the server it spawns and the Playwright run.
   • The feedback suite's beforeEach, before any account or route, refuses PLAYWRIGHT_EXTERNAL_SERVER=1 without that mark. A test pins that only the helper sets it and that the guard comes before set-up.
   • Executed: against a server I started by hand, both tests stop at Feedback E2E refuses a server it did not start.
   • Through node scripts/run-e2e.mjs (port 3108), 2/2 pass.
   • Setting the mark by hand is an operator's word, not a verification, and the helper's comment says so. The isolation claim covers the two managed launch paths only.
3. Synthetic recipients. The provider test now mocks @/lib/constants/super-admins only. The provider gates, the GITHUB_FEEDBACK_REPO setting, and the fetch interception that asserts outbound requests are all the real ones. The positive control asserts both hosts were requested and that the only email recipient was the synthetic admin. The test keeps its filename, which 6096194247 records as the reservation's replacement.

Validation at de21203:

• 19 related suites / 306 tests pass;
• tsc 0; eslint clean; git diff --check clean.
• One environment note: the local Docker daemon had stopped during an idle period and was restarted. The database recovered on its own from an unclean shutdown, and the runner run above was made after that.

---

Generated by Claude Code

[NewWorldVenture]

SEC-007 / new exact-head CI blocker at de21203: CI38052124706 Core job114213197082 FAILED unit step11, independently observed 2026-10-10 12:40:45 UTC. Cause is unknown from metadata; no logs retrieved here. The latest redaction, managed-start qualification and synthetic-recipient changes are accepted in source, but current validation remains blocked. Please diagnose the actual assertion, make the smallest branch-scoped repair without weakening those controls, and publish an exact-head rerun receipt. Keep shared runner ownership and integration prerequisites explicit; no merge/deploy/production actions.

[NewWorldVenture]

Re 6097598207: fixed in b3b1c62.

Failing assertion at de21203: tests/ordering-guards-fail-on-absence.test.ts > no test asserts an order on a bare indexOf, which reported tests/an-e2e-server-files-no-issue-and-sends-no-mail.test.ts:61 and :62. The new check that the managed-server guard comes before createOwnedAccount( and context.route( compared bare indexOf results. A missing guard returns -1, which is less than any real index, so the check would have passed exactly when the guard was deleted. The repository's ratchet refuses that. I found it in a full-suite run on a local integration tree; my pre-push run before de21203 covered related suites only, which was a mistake on my part.

Repair: the three lookups now use at() from tests/helpers/source-order.ts, which asserts presence first. Nothing else changed, and none of the redaction, managed-start or recipient controls were touched.

Exact-head receipt: on b3b1c62, Core job 114216523697 step 11 "Unit tests" ✅. Steps 12–15 and E2E are still running.

Integration prerequisite: once #987 lands, tests/ci-e2e-artifacts.test.ts conflicts textually. The resolution keeps both suites' pinned exclusions, removal checks and shortPrefix entries. On a local tree of current main plus #983–#988 and #990, that file passes 46/46.

---

Generated by Claude Code