fix(metadata): cached file update fails without write access #383
Conversation
This fixes an issue where the metadata cached file cannot be updated because the file was opened as read-only.
WalkthroughThe change modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant doOpenOrCreate
participant FileSystem
Caller->>doOpenOrCreate: Request file access
doOpenOrCreate->>FileSystem: Open file using os.OpenFile(name, os.O_RDWR)
FileSystem-->>doOpenOrCreate: Return file handle or error
alt File does not exist
doOpenOrCreate->>FileSystem: Create new file
FileSystem-->>doOpenOrCreate: Return new file handle or error
else Successful open
doOpenOrCreate->>Caller: Return file handle
end
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
metadata/providers/cached/util.go (3)
28-28: Consider setting appropriate file permissions.The current implementation uses a permission mode of 0, which is too restrictive. Consider using a more appropriate permission mode like 0600 for user read-write access.
- if f, err = os.OpenFile(name, os.O_RDWR, 0); err == nil { + if f, err = os.OpenFile(name, os.O_RDWR, 0600); err == nil {
33-35: Set consistent file permissions for new files.When creating a new file, ensure consistent permissions are set to match the open mode.
- if f, err = os.Create(name); err != nil { + if f, err = os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600); err != nil {
27-41: Consider adding defer to close file on error paths.While the happy path likely has proper file closure handled by the caller, it would be safer to ensure the file is closed on error paths to prevent potential file descriptor leaks.
func doOpenOrCreate(name string) (f *os.File, created bool, err error) { + var file *os.File if f, err = os.OpenFile(name, os.O_RDWR, 0600); err == nil { return f, false, nil } if os.IsNotExist(err) { - if f, err = os.Create(name); err != nil { + if file, err = os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600); err != nil { return nil, false, err } + f = file return f, true, nil } return nil, false, err }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
metadata/providers/cached/util.go(1 hunks)
🔇 Additional comments (1)
metadata/providers/cached/util.go (1)
28-28: LGTM! This fixes the read-only access issue.The change from
os.Opentoos.OpenFilewithos.O_RDWRflag correctly addresses the issue wheredoTruncateCopyAndSeekStartfails to update the cached file due to read-only access.
This fixes an issue where the metadata cached file cannot be updated because the file was opened as read-only.
Version
0.11.2
Description
When initializing a
cached.Providerwithcached.New, if the cached file already exists and needs to be updated, thedoTruncateCopyAndSeekStartmethod will fail because the file was opened as read-only.The error happens here:
webauthn/metadata/providers/cached/util.go
Lines 12 to 14 in 6713911
The cached file is opened as read-only using
os.Open(see here) here:webauthn/metadata/providers/cached/util.go
Lines 28 to 30 in 6713911
How to reproduce
The issue can be reproduced with the following code.
The code will run successfully the first time, when it will download and cache
blob.jwt, and fail on followup runs, trying to truncate the cached file, with the following log error:provider init error:truncate blob.jwt: invalid argument.