Skip to content

Commit

Permalink
Audit: check if context is already cancelled when assessing viability…
Browse files Browse the repository at this point in the history
… for audit (#27531)

* check if context is already cancelled when assessing viability for audit

* changelog
  • Loading branch information
Peter Wilson authored Jun 18, 2024
1 parent 28c2e94 commit 4078417
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
8 changes: 8 additions & 0 deletions audit/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,8 @@ func (b *Broker) IsRegistered(name string) bool {

// isContextViable examines the supplied context to see if its own deadline would
// occur later than a newly created context with a specific timeout.
// Additionally, whether the supplied context is already cancelled, thus making it
// unviable.
// If the existing context is viable it can be used 'as-is', if not, the caller
// should consider creating a new context with the relevant deadline and associated
// context values (e.g. namespace) in order to reduce the likelihood that the
Expand All @@ -472,6 +474,12 @@ func isContextViable(ctx context.Context) bool {
return false
}

select {
case <-ctx.Done():
return false
default:
}

deadline, hasDeadline := ctx.Deadline()

// If there's no deadline on the context then we don't need to worry about
Expand Down
5 changes: 4 additions & 1 deletion audit/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,14 @@ func BenchmarkAuditBroker_File_Request_DevNull(b *testing.B) {
}

// TestBroker_isContextViable_basics checks the expected result of isContextViable
// for basic inputs such as nil and a never-ending context.
// for basic inputs such as nil, cancelled context and a never-ending context.
func TestBroker_isContextViable_basics(t *testing.T) {
t.Parallel()

require.False(t, isContextViable(nil))
ctx, cancel := context.WithCancel(context.Background())
cancel()
require.False(t, isContextViable(ctx))
require.True(t, isContextViable(context.Background()))
}

Expand Down
5 changes: 5 additions & 0 deletions changelog/27531.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
```release-note:bug
core/audit: Audit logging a Vault request/response checks if the existing context
is cancelled and will now use a new context with a 5 second timeout.
If the existing context is cancelled a new context, will be used.
```

0 comments on commit 4078417

Please sign in to comment.