Repository navigation
write the file config store with owner-only permissions - #3411
mrdevel2002-ui wants to merge 2 commits into
Conversation
FileBasedConfigStore wrote config-store.json through a sibling .tmp file which carry resolved credentials, were readable by every local user under the default 022 umask. The .tmp is now created owner-only on POSIX filesystems before it is written and atomically renamed into place.
There was a problem hiding this comment.
🟡 Changes recommended
Concurrent saves share a temporary pathname and can bypass the intended owner-only permissions.
1 open issue
What changed in this PR
This PR hardens the file-backed configuration store used by component management and shared pipes workers to protect persisted credentials from other local users.
Changes:
- Creates temporary store files with owner-only permissions on POSIX filesystems, retaining existing behavior elsewhere.
- Adds a regression test for saved-file permissions, temporary-file removal, and configuration reload.
| File | Description |
|---|---|
| tika-pipes/tika-pipes-core/src/test/java/org/apache/tika/pipes/core/config/ConfigStoreTest.java | Tests owner-only permissions and successful persistence. |
| tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/config/FileBasedConfigStore.java | Applies owner-only permissions before writing and atomically replacing the store. |
Review effort: Balanced
| Path tempFile = configFile.resolveSibling(configFile.getFileName() + ".tmp"); | ||
| Files.deleteIfExists(tempFile); | ||
| Files.createFile(tempFile, ownerOnly()); |
There was a problem hiding this comment.
Fair point, synchronized only covers one instance, and a shared config-store.json.tmp lets a second writer pull the file out from under the first. Fixed in 62b05d1: each save now goes through Files.createTempFile in the store's directory with the owner-only attribute, so every writer gets its own name and nobody else deletes or recreates it. The temp file is also removed if the write or move fails. The test now checks that the directory holds nothing but config-store.json after the saves.
FileBasedConfigStore, the
configStoreType: filebackend behind tika-grpc component management and shared pipes workers, writesconfig-store.jsonthrough a sibling.tmpcreated with the process umask, and the atomic rename publishes those bits. Under the default 022 umask every saved fetcher and emitter config is therefore readable by any other local account, and that store is exactly where resolved credentials end up: S3secretKey, HTTPpasswordandjwtSecret, AzuresasToken, GraphclientSecret(the configuration docs note that a persistent store receives the resolved configurations).saveToFilenow creates the.tmpasrw-------on POSIX filesystems before writing it, so the rename publishes an owner-only file; filesystems without POSIX permissions keep today's behavior. The new test inConfigStoreTestsaves credential-bearing configs through the store and asserts the file is owner-only with no.tmpleft behind; it fails on main (GROUP_READ, OTHERS_READpresent) and passes here, andclean installof tika-pipes-core (221 tests, checkstyle, spotless, forbidden-apis) is green.There is no JIRA issue for this yet; I will open one and add the
TIKA-XXXXprefix to the title and commit if you would like it tracked. I can add the ASFGenerated-by:trailer to the commit if you prefer it there as well.