Enhances core application stability and safety features - #5
Merged
Conversation
Fixed ignored change-requests from PR https://codeberg.org/Conduction/openbuild/pulls/96 Works through the change requests that were raised on #96 but never applied before it merged. Each was re-verified against current code before acting (several claims had gone stale or were inaccurate). Blockers - ApplicationDeletionService — 153 lines of destructive code, no tests: FIXED. Added tests/Unit/Service/ApplicationDeletionServiceTest.php (preserve vs purge, empty-batch, zero-progress, cap-exhaustion). - ApplicationDeletionService purge cap hit silently: FIXED. Log a warning when MAX_PURGE_ROUNDS is exhausted so the downstream register-orphan is diagnosable. Cap kept at 100k (raising it risks request timeout; async deletion is the real large-register fix) — deferred by decision. - AppDeleteDialogSlot / DeleteAppDialog untested: FIXED. Added vitest specs; coverage 85.18% -> 86.07%, ratchet green. - DeleteAppDialog missing i18n strings: MOSTLY STALE. The 4 flagged strings were already in l10n/en.json (added after the review); removed the genuinely-dead "Permanently delete...and data" key. Also extracted 3 unrelated ExportDialog strings that were actually reddening the gate. Concerns - ApplicationPublishControllerTest deleteData forwarding unverified: FIXED. Old mock constrained only 2 of 3 args; now asserts the 3rd positionally (later reworked to drive via the request param, below). - deleteData query-param footgun: HARDENED. The "?deleteData=false -> true" claim is FALSE on NC35 (the dispatcher maps 'false'->false), but the general stringy-bool risk is real, so destroy() now reads the raw param safe-by-default: only '1'/'true' purge, anything else preserves. - einddatum type narrowed to "string": FIXED. Restored ["string","null"] (it was intentionally the one nullable date field; narrowing would fail existing rows holding null on next save). No migration needed. - SeedApplicationTemplates idempotency froze templates on upgrade: FIXED. Implemented version-based upsert (isSeeded rows only, version_compare, admin-created templates never clobbered); added an `updated` count. - builderDesigner route regex maintenance trap: FIXED via guard. Instead of a breaking URL refactor or a (quarantined) Playwright test, added a RoutesTest that derives the designer routes from src/manifest.json and fails in CI if the regex drifts from them. Minor - SeedApplicationTemplates return-on-DoesNotExist skips remaining slugs: DOCUMENTED, no behaviour change. The suggested `continue` would regress the deferred contract; the return is correct because every slug targets one hardcoded schema. Added a comment stating the invariant + when to revisit. - Global v-tooltip directive registration hard to discover: FIXED (docs). Pointer comments in src/main.js and src/registerDirectives.js. - "-dark" slug reservation undocumented: FIXED. Enforced for APP slugs only (schema pattern + SlugValidator::validateAppSlug) and documented; version slugs are deliberately unaffected. Added tests.
…: false` stub) The resolve block sets `fallback.path` to `path-browserify` (needed by the @nextcloud/dialogs FilePicker chunk), but a later block re-assigned `resolve.fallback` with `path: false`, clobbering it. Its comment claimed the FilePicker code path never runs — no longer true: nextcloud-vue's CnFilesWidgetForm opens the folder picker via `pickNodes()`, which calls `path.join`. With `path` stubbed to an empty module, `path.join` was undefined and the picker threw "path.join is not a function" at runtime. Remove the conflicting `path: false` override so `path` stays polyfilled by path-browserify. Production build verified.
rjzondervan
added a commit
that referenced
this pull request
Jul 27, 2026
Address three self-review findings on the harden-rules-authz work: - #3 (L2): MCP admin-bypass audit no longer silently skips when searchObjectsBySlug returns rendered arrays (schema property-auth / _extend / _fields). requireWriteRole re-resolves the app via ObjectService::find() to a hydrated ObjectEntity before the audit write, matching the Copilot path. - #4 (L8): export download authz falls back to @self.owner when the persisted requestedBy is an empty string (a plain ?? treated '' as present and denied the legitimate owner). - #5 (L2): admin_bypass is audited only for a genuine bypass (admin with no owner/editor role); an admin who also holds a real role no longer produces a false compliance record. Applied to both the MCP (AbstractToolHandler) and Copilot (CopilotService) paths. Adds unit tests for each: empty-requestedBy owner fallback, audit-on- rendered-array, and genuine-bypass-only recording (MCP + Copilot). Assisted-by: ClaudeCode:claude-opus-4-8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refines several critical application features, focusing on robustness, safety, and maintainability:
wantsDataPurge()method rigorously validates thedeleteDataflag, avoiding loose boolean coercion for irreversible actions. Includes extensive unit and frontend tests for the deletion flow and dialog.-dark. This prevents routing conflicts with dark-variant icon routes. The rule is enforced via backend validation, an updated JSON schema pattern, and dedicated tests.src/manifest.jsoncorrectly resolve to the designer shell, acting as a durability guard against future route configuration errors.node:pathfor the FilePicker, resolving runtime errors caused bypath.joinbeing undefined.Fixes webpack build issue with node
pathpolyfill.