[feature] Distinct error codes for sm: permission functions (let callers map 403 vs 400)#6464
Open
joewiz wants to merge 1 commit into
Open
[feature] Distinct error codes for sm: permission functions (let callers map 403 vs 400)#6464joewiz wants to merge 1 commit into
joewiz wants to merge 1 commit into
Conversation
sm:chmod / sm:chown / sm:chgrp (and the mode validators) threw the generic ErrorCodes.ERROR for every failure cause -- permission-denied, a malformed mode string, and a mode syntax error were indistinguishable except by message text. An API layer (existdb-openapi, eXide) that wants to map a permission failure to HTTP 403 and an invalid argument to 400 had no stable discriminator to branch on, and resorted to per-operation heuristics. Assign two distinct, stable codes so callers can branch on $err:code: - EXXQDY0007 "Permission denied" (PermissionDeniedException) - EXXQDY0008 "Invalid argument" (bad mode string / mode syntax error) The TransactionException | PermissionDeniedException multi-catch is split so a transaction failure keeps the generic code (a server error, not a 403). Adds permission-error-codes.xqm asserting both codes fire: invalid-argument via sm:has-access (overlong mode) and sm:mode-to-octal (bad syntax); permission-denied via a guest chmod of an admin-only rwx------ resource. SecurityManagerTests: 7/7 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
[This PR was co-authored with Claude Code. -Joe]
Problem
The
sm:permission functions (sm:chmod/sm:chown/sm:chgrp, and the mode validators behindsm:has-access/sm:mode-to-octal) throw the same genericErrorCodes.ERRORfor unrelated failures — permission-denied, a malformed mode string, and a mode syntax error are distinguishable only by message text:catch (PermissionDeniedException) { throw new XPathException(this, e); }throw new XPathException(this, "Mode string must be partial …")catch (SyntaxException se) { throw new XPathException(this, se.getMessage(), se); }XPathException's default error code isErrorCodes.ERROR, and none of these sites override it (the throwable-carrying constructor only adopts a code when the cause is anXPathErrorProvider, whichPermissionDeniedExceptionis not). So all three surface asERROR.An HTTP API layer wants to map a permission failure to 403 Forbidden and an invalid argument to 400 Bad Request, but with one generic code the only discriminator is the message string — brittle (wording can change; no contract). existdb-openapi currently works around it by classifying per operation (a
chmod/chown/chgrpfailure ⇒ 403, aset-mime-typefailure ⇒ 400) rather than inspecting the error. (Surfaced by the existdb-openapi/db-core work.)Change
Assign two distinct, stable codes — following the documented eXist
EXXQDYscheme — so callers can branch on$err:code:EXXQDY0007PermissionDeniedException(chmod/chown/chgrp/ACL, get-permissions)EXXQDY0008The
TransactionException \| PermissionDeniedExceptionmulti-catch in the main dispatch is split, so a transaction failure keeps the generic code (it's a server error → 500, not a 403).Now an API layer maps cleanly:
and existdb-openapi's
db-core:set-permissionscan drop its per-operation heuristic.Files
ErrorCodes.java— addEXXQDY0007/EXXQDY0008in a labeled Security/permission block.PermissionsFunction.java— thread the codes through the four throw sites; split the multi-catch.permission-error-codes.xqm(new XQSuite test, auto-discovered bySecurityManagerTests) — asserts both codes fire: invalid-argument viasm:has-access(overlong mode) andsm:mode-to-octal(bad syntax); permission-denied via a guestsm:chmodof an admin-onlyrwx------resource.Test
SecurityManagerTests— 7/7 green. exist-core builds clean; Codacy PMD clean on both changed Java files.Scope note
This covers the
PermissionsFunctionpermission/mode failures — the ones an HTTP layer most needs to classify. The same generic-code pattern (new XPathException(this, "message")with no code) is pervasive across the rest of thesm:module (AccountManagement,AccountStatus,FindGroup, …); extending distinct codes there is a natural follow-up. If the team would prefer a dedicated security code family over reusingEXXQDY, that's an easy rename before merge.