Skip to content

fix(desktop): propagate canonical deep-link length validation - #1982

Merged
integry merged 1 commit into
1950-epic-cross-platform-dskfrom
1952-epic-electron-runtime-app
Aug 29, 2026
Merged

fix(desktop): propagate canonical deep-link length validation#1982
integry merged 1 commit into
1950-epic-cross-platform-dskfrom
1952-epic-electron-runtime-app

Conversation

@integry

@integry integry commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Propagates the final canonical deep-link serialization invariant from the runtime epic into the top desktop epic.

  • reject canonical hrefs that expand beyond 2,048 characters
  • revalidate the exact serialized value delivered to renderer
  • require raw/canonical dashboard path equivalence
  • add desktop and renderer boundary regressions

This PR targets the non-main desktop epic branch. It must be reviewed and merged only into that branch. Related: #1950 #1952 #1979.

…8dcb68971e6a0e6`.

Implemented F1 only on base head `48f85811addf4bccfd390961e8dcb68971e6a0e6`.

Changes:

- [security.ts](/tmp/git-processor/worktrees/integry/propr/pr-1979-followup-2026-08-29T20-41-29/apps/desktop/src/security.ts) now checks the serialized canonical candidate’s length/controls and requires its dashboard path to equal the validated input path before returning it.
- Added desktop and renderer regressions for percent-expansion beyond 2,048 and an exactly 2,048-character valid canonical link.
- Existing traversal, encoded-delimiter, auth/pairing, query, and hash tests remain passing.

Verification:

- Desktop tests: 28 passed.
- Focused renderer tests: 5 passed.
- Desktop and UI typechecks: passed.
- Production package: passed.
- `git diff --check`: passed.
- Packaged smoke: environment-blocked because no usable Chromium sandbox helper is installed/configured.
- Full suite: reached file 155/320 before blocking on unavailable Redis (`127.0.0.1:6379`); neither Redis nor Docker is installed.

Only the three scoped files are modified. Per instruction, I did not commit or push; the system must publish the resulting branch head.

PR: #1979
Comment by: @integry (ID: 5464771154)
Model: gpt-5.6-sol
@integry

integry commented Aug 29, 2026

Copy link
Copy Markdown
Owner Author

/review

Review exact head d99f006 propagated from the runtime epic into top epic branch 1950. Verify the three-file delta is exactly the canonical deep-link serialization fix plus focused regressions, preserves every top-epic runtime/UX change, and introduces no semantic conflict. Reproduce canonical length boundaries and re-audit encoded traversal/auth/reserved/external rejection. No code changes. Report blockers and score only after exact-head checks.

@propr-dev

propr-dev Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@integry integry removed the AI label Aug 29, 2026
@propr-dev

propr-dev Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

At exact head d99f006211026dac64d355ceb9d5c31d2683b22b, the three-file delta is narrowly limited to canonical deep-link serialization validation and focused regressions. The 2,048-character boundary is correctly enforced on the serialized value, raw/canonical dashboard routes must agree, and traversal, credential/auth, reserved-parameter, and external-route rejection remain intact. No semantic conflict with the surrounding runtime or UX behavior is evident.

The code itself has no verified merge blocker, but the PR needs minor follow-up before merge because the authoritative current status includes a failed Validate Changes check and two pending full-suite runs.

Canonical value is revalidatedapps/desktop/src/security.ts checks the exact url.href length and control characters, reparses it, and requires its normalized dashboard path to equal the raw input’s normalized path.

Length boundaries are correctly covered — Both desktop and renderer regressions demonstrate rejection when serialization expands beyond 2,048 characters and acceptance at exactly 2,048.

Security invariants remain preserved — The changed path continues through the existing credential, encoded traversal, reserved route/parameter, and absolute/external destination checks.

Merge blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The scoped implementation appears correct and well tested, but the current failed validation check prevents declaring the exact head merge-ready until its cause is resolved; the pending full-suite checks should also complete.

Score: 7/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 1m 7s
  • Tokens: 25,671 (23,275 in / 2,396 out)
  • Cost: $0.13

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@integry integry added the AI label Aug 29, 2026
@integry

integry commented Aug 29, 2026

Copy link
Copy Markdown
Owner Author

/review

Re-review exact unchanged head d99f006 now that all nine exact-head checks, including rerun Validate Changes and Full Test Suite, are green. Confirm prior no-blocker canonical deep-link assessment, exact three-file propagation into top epic, 673/674 serialization boundaries, and preserved security/runtime behavior. No code changes. Report final blockers and score from current evidence.

@propr-dev

propr-dev Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@integry integry removed the AI label Aug 29, 2026
@propr-dev

propr-dev Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This exact three-file delta is focused, correct, and ready to merge into the intended non-main desktop epic branch.

Canonical serialization enforcednormalizeDeepLink validates the serialized url.href, rejects values exceeding 2,048 characters, and revalidates open-link path equivalence.

Boundary behavior is correct — The 2,048-character case is accepted while the next expansion beyond the limit is rejected, covering the intended 673/674 serialization transition.

Security behavior preserved — Existing traversal, authentication, reserved-parameter, malformed, and external-path rejection remains intact; production renderer behavior is unchanged. Current-head status reports 0 failed, 0 pending, and 27 passed checks.

Merge blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The implementation fully satisfies the scoped objective, adds focused desktop and renderer regressions, and has no identified behavioral or security regressions at a fully green current head.

Score: 10/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 56s
  • Tokens: 25,344 (23,058 in / 2,286 out)
  • Cost: $0.14

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@integry
integry merged commit 7d830b9 into 1950-epic-cross-platform-dsk Aug 29, 2026
82 of 83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant