Skip to content

Harden WebDAV opaque source contract - #253

Merged
seonghobae merged 2 commits into
masterfrom
feature/webdav-opaque-source-20260528
May 28, 2026
Merged

Harden WebDAV opaque source contract#253
seonghobae merged 2 commits into
masterfrom
feature/webdav-opaque-source-20260528

Conversation

@seonghobae

@seonghobae seonghobae commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep WebDAV account primary keys internal by making legacy source_uid backfill non-sequential and not account_id-derived
  • strengthen WebDAV API tests for signed bearer sessions, public identity-header rejection, and account_id omission across account/writeback/materialization responses
  • make the Data WebDAV intent CTA wait for a confirmed writable opaque source_id before posting target_source_id
  • update README, AGENTS, and source-of-truth docs with the opaque WebDAV contract

Verification

  • PYTHONDONTWRITEBYTECODE=1 DISABLE_BACKGROUND_WORKERS=1 python3 -m pytest backend/tests/test_webdav_api.py backend/tests/test_bootstrap_db.py -q
  • cd frontend && npx vitest run src/app/data/page.test.tsx src/app/tasks/page.test.tsx
  • cd frontend && npm run lint -- src/components/DataLayout.tsx src/components/TasksLayout.tsx src/app/data/page.test.tsx src/app/tasks/page.test.tsx tests/e2e/dashboard-branding.spec.ts tests/e2e/helpers.ts
  • cd frontend && npm run typecheck
  • cd frontend && env -u NO_COLOR PLAYWRIGHT_PORT=18194 npm run test:e2e -- tests/e2e/dashboard-branding.spec.ts -g "data WebDAV|self-sent knowledge WebDAV" --project=desktop
  • cd frontend && NEXT_STATIC_GENERATION_MAX_CONCURRENCY=1 NEXT_STATIC_GENERATION_MIN_PAGES_PER_WORKER=100 npm run build

Screenshot evidence inspected

  • data-webdav-writeback-intent-desktop.png
  • data-webdav-writeback-intent-mobile.png
  • data-webdav-writeback-intent-mobile-scroll.png
  • self-sent-knowledge-webdav-intent-desktop.png
  • self-sent-knowledge-webdav-intent-mobile.png
  • self-sent-knowledge-webdav-intent-mobile-scroll.png

Notes

  • Strix remains direct OpenAI Platform only. Do not use GitHub Models.

Summary by CodeRabbit

  • New Features

    • WebDAV writeback now uses opaque browser-visible source IDs (not sequential account IDs).
    • WebDAV endpoints accept signed bearer session authentication.
    • UI: improved WebDAV account loading, explicit fetch_error/no_source states, and writeback button disabling while loading.
  • Tests

    • Added/updated tests to assert signed-session flows, omission of internal account IDs from responses, and UI behavior when account loading fails.
  • Documentation

    • Clarified WebDAV writeback scope, eligibility, and source identifier contract.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c11cf9c2-25b5-420f-a6a3-19fe67d198ed

📥 Commits

Reviewing files that changed from the base of the PR and between a89f0a9 and 018ff53.

📒 Files selected for processing (2)
  • frontend/src/app/data/page.test.tsx
  • frontend/src/components/DataLayout.tsx

📝 Walkthrough

Walkthrough

Implements opaque browser-visible WebDAV source IDs by backfilling webdav_accounts.source_uid with non-deterministic values, updates tests and endpoints to require signed bearer sessions and omit account_id, and modifies frontend state and docs to handle account-list readiness and new identifiers.

Changes

WebDAV Opaque Source ID

Layer / File(s) Summary
Source UID backfill implementation
backend/scripts/bootstrap_db.py, backend/tests/test_bootstrap_db.py
Backfill SQL now derives webdav_accounts.source_uid from random(), clock_timestamp(), user_id, and server_url instead of account_id. Tests updated to assert the new expression includes random() and clock_timestamp().
API authentication and payload contract
backend/tests/test_webdav_api.py
Adds signed-session test helpers (_request_with_signed_session, _request_without_signed_session), switches tests (including real-DB smoke test) to Authorization Bearer signed sessions, and adds assertions that /accounts, /writeback-intent, and /knowledge-materialization-intent responses omit account_id and include source_id.
Frontend integration and UI state management
frontend/src/components/DataLayout.tsx
Adds webdavAccountStatus lifecycle state, getSafeErrorSummary logging helper, short-circuits writeback intent when accounts are not ready, selects and posts target_source_id, disables the intent-check button during loading, and renders explicit account-list error and fetch_error messages.
Frontend tests and test hygiene
frontend/src/app/data/page.test.tsx
Restores mocks after each test and adds a test that simulates /api/webdav/accounts failure to verify UI disabling, no intent call, and safe error logging without leaking session tokens.
Documentation and governance updates
AGENTS.md, README.md, docs/operations/source-of-truth-and-writeback-sovereignty.md, docs/plans/2026-05-27-webdav-opaque-source-id.md
Clarifies that browser-visible WebDAV identifiers are webdav_accounts.source_uid (opaque, unique), forbids exposure or deterministic derivation from sequential account_id, scopes lookup by signed-session organization_id, and requires rejecting legacy target_account_id payloads.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • Seongho-Bae/naruon#242: Overlaps on signed Bearer-session authorization changes and self-sent knowledge WebDAV intent tests.
  • Seongho-Bae/naruon#247: Related prior work on switching from numeric account_id exposure to opaque source_uid identifiers and associated backfill logic.

Poem

🐰 A rabbit's note on hidden keys

No numeric keys for eyes to meet,
source_uid keeps secrets neat.
Random hops and timestamps blend,
Tokens signed, session's friend.
Account IDs tucked out of sight.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Harden WebDAV opaque source contract' directly and clearly summarizes the main change: strengthening the WebDAV contract by making source identifiers opaque and non-sequential, which is confirmed across all file changes from documentation to backend logic to frontend UI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/webdav-opaque-source-20260528

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 018ff53ff58aad6ecb32d77d3417e67f5f161c66:

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/components/DataLayout.tsx`:
- Around line 111-114: The click handler currently checks only targetSourceId
and sets setWritebackStatus('no_source'), which hides a failed
/api/webdav/accounts fetch; update the guard to first check webdavAccountStatus
and if it's not 'ready' keep the writeback in the fetch-error state (e.g.,
setWritebackStatus('fetch_error') or return early) instead of setting
'no_source'; do this in the handler referencing targetSourceId and
setWritebackStatus, and also update the CTA disabled logic to include
webdavAccountStatus !== 'ready' (or specifically webdavAccountStatus ===
'errored') so the action stays disabled while accounts are errored — apply the
same change in the other occurrences mentioned (the blocks around the symbols at
the other locations).
- Around line 91-99: Replace the raw console.error(error) call in the WebDAV
fetch promise catch with a static non-secret label and a sanitized error
summary: do not log the whole error object from the fetch, instead extract only
non-sensitive fields (e.g., HTTP status or a short, fixed-length message) and
pass those values with a constant label like "WebDAV fetch error" to
console.error; update the catch block that currently calls setWebdavAccounts([])
and setWebdavAccountStatus('error') (the promise chain that calls
setWebdavAccounts and setWebdavAccountStatus) to perform this sanitized logging
so no signed-session or credential-derived data is emitted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 402ff4db-656a-441b-a0f2-c5f725879e36

📥 Commits

Reviewing files that changed from the base of the PR and between 8c0b85c and a89f0a9.

📒 Files selected for processing (8)
  • AGENTS.md
  • README.md
  • backend/scripts/bootstrap_db.py
  • backend/tests/test_bootstrap_db.py
  • backend/tests/test_webdav_api.py
  • docs/operations/source-of-truth-and-writeback-sovereignty.md
  • docs/plans/2026-05-27-webdav-opaque-source-id.md
  • frontend/src/components/DataLayout.tsx

Comment thread frontend/src/components/DataLayout.tsx
Comment thread frontend/src/components/DataLayout.tsx
@seonghobae

Copy link
Copy Markdown
Contributor Author

Temporary Strix required-context handling for head 018ff53.

Evidence captured on 2026-05-28:

Per the repository merge-gate policy, I am temporarily removing only the required strix context, merging normally without admin bypass, and restoring strix immediately after merge.

@seonghobae
seonghobae merged commit c94a248 into master May 28, 2026
15 of 16 checks passed
@seonghobae
seonghobae deleted the feature/webdav-opaque-source-20260528 branch May 28, 2026 01:45
@seonghobae

Copy link
Copy Markdown
Contributor Author

Post-merge restoration complete.

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