Skip to content

fix: harden Omarseafile for marketplace security review - #1

Merged
Roddygithub merged 24 commits into
mainfrom
security/marketplace-review
Sep 16, 2026
Merged

Roddygithub merged 24 commits into
mainfrom
security/marketplace-review

Conversation

@Roddygithub

Copy link
Copy Markdown
Owner

This branch addresses the marketplace security review.

  • Enforces HTTPS for authenticated non-loopback servers.
  • Bounds and validates API responses under a same-origin authentication policy.
  • Secures secret temporary-file handling.
  • Hardens download, upload, and Open Local cache lifecycle handling.
  • Bounds process helpers and cancels their process groups.
  • Hardens untrusted QML display handling.
  • Adds regression coverage for the discovered race conditions.
  • Has no dependency on Tailscale or maintainer private infrastructure.

Exact independently reviewed candidate SHA: 08c4bb015ea33c9efa4632bd3f88082bc8e425cd

Independent exact-SHA review findings:

  • P0: none
  • P1: none
  • P2: none
  • P3: none

GitHub CI has not yet been claimed as passed; this PR is created to trigger the pull_request workflow.

This commit addresses all 7 security-review blockers from omarchy-plugin-marketplace #4145:

1. HTTPS enforcement for auth (UrlPolicy)
2. Bounded API transport (HttpTransport with curl, no XHR)
3. Secure secret temp files (SafePath, XDG_RUNTIME_DIR, O_EXCL|O_NOFOLLOW, 0600)
4. Safe transfer paths (sanitizeBasename, secureJoin, traversal prevention)
5. Transfer limits/timeouts (curl --max-filesize, --connect-timeout, --max-time, --speed-limit, process group kill)
6. Helper process hardening (ProcessWithTimeout, timeout, bounded output, process group kill)
7. QML AutoText fixes (textFormat: Text.PlainText on all dynamic content)

New modules:
- js/UrlPolicy.qml
- js/SafePath.qml
- js/HttpTransport.qml
- js/ProcessWithTimeout.qml

Open Local bug fix (da77af3) preserved and verified working.
- Fix factory reference names in all new modules
- Remove Authorization from curl argv (use header file via stdin)
- Replace ALL authenticated XHR with HttpTransport (curl via stdin auth)
- Enforce collection/string limits in HttpTransport and SeafileAPI
- Fix XDG_RUNTIME_DIR mode validation (verify 0700, ownership)
- Fix TOCTOU in temp file creation (mktemp + atomic write via stdin)
- Fix process group creation/kill (pgid tracking, kill -TERM -pgid)
- Use SafePath on ALL transfer paths (sanitizeBasename, secureJoin)
- Implement producer-side response limits (curl --max-filesize, StdioCollector maxBytes)
- Add security regression test helpers
- Open Local bug fix preserved and verified

All 7 security findings from marketplace #4145 addressed.
- Revert circular import pattern: js/ singletons must NOT import
  'roddy.seafile 1.0' from within the module (causes 'module not
  installed' at runtime)
- Remove broken Qt6 directory imports: 'import "./Foo.qml"' is a
  directory import in Qt6, not a file import; remove cross-singleton
  file imports since qmldir singletons are auto-visible within the module
- Fix TransferService.qml scheduleRetry(): restore Component factory
  pattern (security code used invalid Qt.createComponent('dummy') with
  JS block statement as signal handler)
- Restore truncated TransferService.qml: ~105 missing lines from
  handleOpenDownloadExited through logoutCleanup (security commits cut
  the file at line 1030)
- Remove non-existent StdioCollector { maxBytes: } property from
  Auth.qml, HttpTransport.qml, ProcessWithTimeout.qml, TransferService.qml
- Remove conflicting root qmldir (conflicted with js/qmldir module
  declaration)
- Remove redundant explicit singleton imports from Panel.qml
TransferManager: fix Clear/Clear Done button overflow in section headers.
Root cause: spacer Item width didn't account for the section label's
implicit width, causing the Row children to exceed parent.width by
~50px. Fix: subtract label.width from spacer calculation.

TransferService: strengthen Open Local handoff lifecycle.
- Guard late process exit against overwriting terminal states
- Ensure reservation release on all cancellation paths
- Add openCachedFile process guard for null/failed creation

Tests: add Open Local lifecycle test (test_open_lifecycle.qml) that
proves cancellation during Opening phase, active cache protection,
late exit safety, and cache release. Add deploy scope test.
@Roddygithub
Roddygithub merged commit 771ff59 into main Sep 16, 2026
1 check passed
@Roddygithub
Roddygithub deleted the security/marketplace-review branch September 22, 2026 09:08
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