Found while reviewing #6046.
Problem
NewWorkflowAuditor opens the audit log through config.GetLogWriter():
// pkg/audit/workflow_auditor.go:33
logWriter, err := config.GetLogWriter()
When Config.LogFile is set, GetLogWriter returns an os.OpenFile handle (pkg/audit/config.go:69-81). But WorkflowAuditor has no Close method — the HTTP Auditor does:
// pkg/audit/auditor.go:109-114
func (a *Auditor) Close() error {
if closer, ok := a.logWriter.(io.Closer); ok {
return closer.Close()
}
return nil
}
So a file-backed WorkflowAuditor holds the descriptor for the process lifetime with no way for the owner to release it. WorkflowAuditor does not even retain the writer, so the fd is unreachable once the constructor returns.
Impact
One leaked fd per WorkflowAuditor instance. Bounded and low severity if the auditor is a long-lived singleton; it matters if one is ever constructed per workflow, per reconcile, or in tests that build many.
Suggested fix
Add Close() error mirroring Auditor.Close() (which means retaining the writer on the struct), and call it wherever the workflow auditor's lifetime ends.
While there, note a pre-existing hazard worth guarding in both types: with LogFile empty, GetLogWriter returns os.Stdout, which satisfies io.Closer — so Close() on a stdout-backed auditor closes fd 1. Auditor.Close() has this today; a new WorkflowAuditor.Close() should not inherit it. Skipping the close when the writer is os.Stdout fixes both.
Found while reviewing #6046.
Problem
NewWorkflowAuditoropens the audit log throughconfig.GetLogWriter():When
Config.LogFileis set,GetLogWriterreturns anos.OpenFilehandle (pkg/audit/config.go:69-81). ButWorkflowAuditorhas noClosemethod — the HTTPAuditordoes:So a file-backed
WorkflowAuditorholds the descriptor for the process lifetime with no way for the owner to release it.WorkflowAuditordoes not even retain the writer, so the fd is unreachable once the constructor returns.Impact
One leaked fd per
WorkflowAuditorinstance. Bounded and low severity if the auditor is a long-lived singleton; it matters if one is ever constructed per workflow, per reconcile, or in tests that build many.Suggested fix
Add
Close() errormirroringAuditor.Close()(which means retaining the writer on the struct), and call it wherever the workflow auditor's lifetime ends.While there, note a pre-existing hazard worth guarding in both types: with
LogFileempty,GetLogWriterreturnsos.Stdout, which satisfiesio.Closer— soClose()on a stdout-backed auditor closes fd 1.Auditor.Close()has this today; a newWorkflowAuditor.Close()should not inherit it. Skipping the close when the writer isos.Stdoutfixes both.