security: make one-time downloads atomic under concurrency (ai:gpt-5) #49

New issue

Closed

opened 2026-05-23 21:23:07 +02:00 by heiko · 1 comment

heiko commented

2026-05-23 21:23:07 +02:00

Owner

Finding from a whole-codebase security review.

Affected code:

cmd/once-server/download.go:65-83 opens content, then removes it.
cmd/once-server/store/file.go:67-80 removes content and updates metadata after the caller already has an open reader.

Impact:
Two concurrent POST requests for the same download key can both open the content file before either request removes it. Both requests then keep valid file handles and can receive the secret, violating the documented exactly-once guarantee.

Suggested fix:

Claim a download atomically before streaming, for example by renaming content to a per-download in-progress path with os.Rename, or by using an advisory lock/claim file in the UUID directory.
Treat an already-claimed or already-removed object as downloaded.
Update metadata consistently with the successful claim.
Add a concurrency test that fires two POST downloads at the same key and asserts that only one gets the content.

Finding from a whole-codebase security review. Affected code: - cmd/once-server/download.go:65-83 opens `content`, then removes it. - cmd/once-server/store/file.go:67-80 removes `content` and updates metadata after the caller already has an open reader. Impact: Two concurrent POST requests for the same download key can both open the content file before either request removes it. Both requests then keep valid file handles and can receive the secret, violating the documented exactly-once guarantee. Suggested fix: - Claim a download atomically before streaming, for example by renaming `content` to a per-download in-progress path with `os.Rename`, or by using an advisory lock/claim file in the UUID directory. - Treat an already-claimed or already-removed object as downloaded. - Update metadata consistently with the successful claim. - Add a concurrency test that fires two POST downloads at the same key and asserts that only one gets the content.

heiko referenced this issue from a commit

2026-05-23 23:07:44 +02:00

fix: make one-time downloads atomic (ai:gpt-5)

heiko referenced this issue

2026-05-23 23:09:23 +02:00

fix: make one-time downloads atomic (ai:gpt-5) #56

heiko added the

security

label

2026-05-24 18:44:09 +02:00

heiko commented

2026-05-24 20:15:50 +02:00

Author

Owner

Fixed by commit 4e53e38 (fix: make one-time downloads atomic).

Implements atomic claiming via File.Claim() method which uses atomic os.Rename():

Renames content → content.download atomically
Opens the claimed file
Deletes the claimed marker

Prevent TOCTOU vulnerabilities and concurrent downloads of the same file. Verified with test coverage (TestClaimAllowsOnlyOneDownloader).

Fixed by commit 4e53e38 (fix: make one-time downloads atomic). Implements atomic claiming via File.Claim() method which uses atomic os.Rename(): 1. Renames `content` → `content.download` atomically 2. Opens the claimed file 3. Deletes the claimed marker Prevent TOCTOU vulnerabilities and concurrent downloads of the same file. Verified with test coverage (TestClaimAllowsOnlyOneDownloader).

heiko closed this issue

2026-05-24 20:15:59 +02:00