fix(settings): write the execution audit log through the app's storage environment - #2246
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Fixes #2242.
ExecutionAuditLog.defaultFileURL()askedFileManagerfor Application Support directly instead of going throughAppStorageEnvironment.shared, which is the one type allowed to resolve that directory.Two things follow from that, and both are fixed here.
swiftlint --strictfailed onmain. The repo's ownstorage_environment_directoryrule isseverity: error, so the mandatory lint step was red for everyone on a violation they did not write. A gate that is always red teaches people to ignore it.UI tests wrote to the developer's own audit log.
TableProUITestslaunches the real app against a sandbox named inTABLEPRO_UI_TEST_SANDBOX, and every other store honours it. This path did not, so a test run appended to the log of whoever ran it. The log is hash chained, so those entries are not noise: they are indistinguishable from real decisions and they advance the chain.The change
defaultFileURL()now readsAppStorageEnvironment.shared.supportDirectory. It also drops to a non-optionalURL: the old signature returnedURL?only becauseFileManager.url(for:create:)could throw, and the environment cannot fail to name a root. That removes two unreachableguard let fileURLunwraps inload()andpersist().The production path is unchanged, byte for byte.
AppStorageEnvironment.productionRoot()resolves the same directory the old call did, so there is no migration and no existing log moves.Verification
swiftlint --strictover the whole app target: exit 0, no output. It reports this exact violation on the unfixed tree, so that is a clean A/B.ExecutionAuditRecordTests,ExecutionAuditLogTests,AppStorageEnvironmentTests: 20 executed, 20 passed. Log grepped forFailing tests:andTEST FAILED, 0 hits.Test added
defaultLocationFollowsStorageEnvironmentasserts the log resolves insideAppStorageEnvironment.shared.supportDirectory. It is a narrow guard and the lint rule remains the primary one: this test catches a future relocation outside the sandboxed root, the lint rule catches a return to resolving the directory by hand.No CHANGELOG entry
Deliberate. CLAUDE.md asks for one-line user-facing entries, and this has no user-visible effect: same path, same contents, same behaviour. The bug only reaches someone running the test suite.
Noted, not fixed here
AppStorageEnvironment.supportDirectoryhad zero readers before this commit. Fifteen storage classes hand-append"TablePro"toapplicationSupportRootinstead, which is the duplication that property exists to remove. Mechanical to fix, but it touches fifteen files that other branches are editing, so it does not belong in a bug fix.