Skip to content

Restore PHP source excerpts in embedded v2 exception details - #248

Merged
rmcdaniel merged 5 commits into
mainfrom
fix/247-embedded-exception-source
Oct 25, 2026
Merged

rmcdaniel merged 5 commits into
mainfrom
fix/247-embedded-exception-source

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

Embedded v2 currently displays trace_preview as PHP source. This restores local source excerpts in authorized embedded full run details, using separate typed source_context and trace_preview fields. The exception view highlights the actual throwing line, labels missing source as unavailable, and keeps trace inspection available even without source.

Source defaults to canonical app_path() only, with explicit additional roots and WATERLINE_SOURCE_CONTEXT=false to disable it. Reads enforce file/line/excerpt and response budgets and stable regular PHP files; unauthenticated access and service mode cannot read source. Full-detail enrichment is explicit, so bounded observations, exports and internal diagnostic projections never receive excerpts. Responses prevent caching, source and metadata use Vue text escaping, and workflow failure/history rows remain unchanged. No SDK source capture, transmission or storage is added.

Validation through current head a347af1f04e1527176a7cbebc3e368dcef7b1699:

  • PHP regression tests: 10 tests / 139 assertions pass, including source near file boundaries, forged cached config, wrappers/traversal/symlink escapes/unreadable/nonregular files, budgets, auth/service/disable gates, typed history and failure fallback, trace preservation and no persisted excerpt.
  • Full frontend suite: 136/136 passed before the final region-label markup; the changed component's 3/3 tests and production build also pass at the current head.
  • Built production interface with actual authorized API fixtures: 24/24 Chromium cases pass across dark/light themes and 1440/768/390 widths, throw lines 1/5/10 and source unavailable. Checks include the visible numeric highlight, escaped hostile markup, separate preserved trace, long source lines in scroll regions and no page overflow. Mobile screenshots inspected.
  • Three Sonar maintainability findings are fixed. Two tabindex warnings were reviewed as false positives for keyboard-scrollable overflow regions, with direct keyboard proof and W3C guidance recorded on the findings. Two semantic-role findings are fixed by native named sections. The final Sonar analysis is running; no global rule is disabled.
  • Local full SQLite initially had 8 environment failures among 729 tests (missing Node and inaccessible Git worktree metadata). Adding Node resolved seven; one archive-metadata check remained in that minimal environment. The normal hosted SQLite job subsequently passed at prior head 75c16b1; the full hosted matrix for the current head is running.

Security independently passed the source at 095fbfc and the label-only delta at 4c2af1d (review). The final presentation-only native-section delta is supplied for confirmation. Four additional dark/light mobile cases at the current head pass, including ArrowRight scrolling and Tab navigation from Source to Trace. Publication and the fixed published-package rendered acceptance remain required after the existing #241 protected publisher cutover. No tag or publisher dispatch is queued. These source/browser checks do not claim published delivery.

Refs #247. Keep it open through publication and rendered acceptance.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security source-review disposition for exact head 095fbfc84adbe89ae3feb2bda4cee6be840f9dd3: PASS. I found no remaining security blocker in this implementation.

The reviewed path is limited to authorized embedded full-detail responses; service mode, unauthenticated mode, bounded observations, exports, and ordinary resource resolution remain source-free. Canonical allowlisted PHP paths, regular-file and stable-inode checks, per-file/read/excerpt/response bounds, raw-text Vue rendering, and private, no-store responses cover the agreed disclosure and resource-exhaustion risks. Source remains ephemeral and separate from the trace; no history/failure persistence or SDK collection was added. I also checked the hostile-path/file and hostile-markup regression coverage and the exact diff against the accepted issue scope.

This is a source-level security disposition only. Normal CI, rendered browser qualification, publication, and installed published-artifact acceptance for #247 still remain with the product owner; do not cite this review as proof of those stages.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security reviewed the post-disposition delta from 095fbfc84adbe89ae3feb2bda4cee6be840f9dd3 through exact head 4c2af1de41dc7ecc85f84126558355274daf058c. It changes only the source/trace scroll regions' localized accessibility roles/labels and rebuilt generated assets; no PHP reader, authorization, path, response, persistence, or disclosure behavior changed. The source-level Security PASS carries forward to 4c2af1de41dc7ecc85f84126558355274daf058c. Normal CI, publication, and installed-artifact acceptance remain separate.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review carries PASS to exact head a347af1f04e1527176a7cbebc3e368dcef7b1699.

I reviewed the exact delta from previously accepted 4c2af1de41dc7ecc85f84126558355274daf058c. The authored change is presentation-only: the two keyboard-scroll regions now use named native <section> elements containing the existing <pre>/<code> content, with a small scoped style adjustment; the remaining changed files are rebuilt frontend assets. Vue text interpolation continues to escape the excerpt and trace, and the delta does not change PHP source retrieval, authorization, path validation, persistence, or response bounds.

No new security finding in this delta. This source disposition does not replace General's final CI, publication, or installed-artifact acceptance.

@rmcdaniel
rmcdaniel marked this pull request as ready for review October 25, 2026 10:24
@rmcdaniel
rmcdaniel merged commit fe48bd3 into main Oct 25, 2026
37 checks passed
@rmcdaniel
rmcdaniel deleted the fix/247-embedded-exception-source branch October 25, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant