Skip to content

Fix session recording truncation#60891

Merged
Joerger merged 7 commits intomasterfrom
joerger/fix-audit-disk-truncated
Nov 3, 2025
Merged

Fix session recording truncation#60891
Joerger merged 7 commits intomasterfrom
joerger/fix-audit-disk-truncated

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Oct 31, 2025

Changelog: fix an issue which could lead to session recordings saved on disk being truncated.

Fixes #60860

@github-actions github-actions bot added audit-log Issues related to Teleports Audit Log size/sm labels Oct 31, 2025
@github-actions github-actions bot requested review from eriktate and gzdunek October 31, 2025 21:10
Comment thread lib/events/filesessions/filestream_test.go Outdated
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
@Joerger Joerger enabled auto-merge October 31, 2025 22:50
@Joerger Joerger disabled auto-merge October 31, 2025 23:11
Comment on lines -131 to -138
{
desc: "OnlyReservation",
expectedContent: []byte{},
partsFunc: func(t *testing.T, handler *Handler, upload *events.StreamUpload) {
createPart(t, handler, upload, int64(1), []byte{})
createPart(t, handler, upload, int64(2), []byte{})
},
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rosstimothy FYI I had to remove this test as it was basically ensuring that we do upload empty uploads if we find an empty upload instead of discarding it.


gotContent, err := io.ReadAll(f)
require.NoError(t, err)
require.Equal(t, content, gotContent)
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's take a closer look at this fix since test is failing in CI.

        	Error:      	Not equal: 
        	            	expected: []byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}
        	            	actual  : []byte{}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,3 +1,2 @@
        	            	-([]uint8) (len=11) {
        	            	- 00000000  68 65 6c 6c 6f 20 77 6f  72 6c 64                 |hello world|
        	            	+([]uint8) {
        	            	 }
        	Test:       	TestCleanupEmptyUpload

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was caused by 9177f92, reverted. Sorry for the haphazard change, in addition to being done wrong, I don't think it's necessary.

@Joerger Joerger requested a review from rosstimothy November 3, 2025 17:00
@Joerger Joerger enabled auto-merge November 3, 2025 18:47
@Joerger Joerger added this pull request to the merge queue Nov 3, 2025
Merged via the queue into master with commit 1d9da13 Nov 3, 2025
41 checks passed
@Joerger Joerger deleted the joerger/fix-audit-disk-truncated branch November 3, 2025 19:30
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@Joerger See the table below for backport results.

Branch Result
branch/v17 Failed
branch/v18 Create PR

Joerger added a commit that referenced this pull request Nov 3, 2025
* Cleanup empty uploads rather than completing them.

* Add regression test.

* Update lib/events/filesessions/filestream_test.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Remove test for uploading an empty reservation upload.

* Cleanup after FS lock.

* Revert "Cleanup after FS lock."

This reverts commit 9177f92.

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this pull request Nov 8, 2025
* Cleanup empty uploads rather than completing them.

* Add regression test.

* Update lib/events/filesessions/filestream_test.go



* Remove test for uploading an empty reservation upload.

* Cleanup after FS lock.

* Revert "Cleanup after FS lock."

This reverts commit 9177f92.

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
mmcallister pushed a commit that referenced this pull request Nov 19, 2025
* Cleanup empty uploads rather than completing them.

* Add regression test.

* Update lib/events/filesessions/filestream_test.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Remove test for uploading an empty reservation upload.

* Cleanup after FS lock.

* Revert "Cleanup after FS lock."

This reverts commit 9177f92.

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
mmcallister pushed a commit that referenced this pull request Nov 20, 2025
* Cleanup empty uploads rather than completing them.

* Add regression test.

* Update lib/events/filesessions/filestream_test.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Remove test for uploading an empty reservation upload.

* Cleanup after FS lock.

* Revert "Cleanup after FS lock."

This reverts commit 9177f92.

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session Upload Completer wipes recent session recordings every 24 hours

3 participants