Skip to content

Commit a98c0c6

Browse files
committed
fix: open log file with mode 0600 and remove permission widening fallback
The agent runs as root, so if it cannot open its own log file, chmod/chown workarounds won't help — just fail. Removed the fallback that ran chmod 666 which made the log world-writable. Now opens with 0600 and returns the error directly on failure.
1 parent 5f2fd86 commit a98c0c6

File tree

2 files changed

+4
-61
lines changed

2 files changed

+4
-61
lines changed

components/arc/v20260301/arc_registration.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ func (a *installArcAction) addAuthenticationArgs(ctx context.Context, args *[]st
169169
// The token is short-lived (~60 minutes) which limits the exposure window, but it is
170170
// still observable during the registration process. azcmagent does not currently support
171171
// reading the token from stdin or an environment variable.
172-
// Consider switching to --service-principal-cert or --use-azcli when feasible.
173-
// See: https://learn.microsoft.com/en-us/azure/azure-arc/servers/azcmagent-connect
172+
// TODO: Check if we can let azcmagent discover auth settings on its own (e.g. --use-azcli
173+
// or VM MSI) instead of fetching a token ourselves and passing it on the command line.
174174
*args = append(*args, "--access-token", accessToken)
175175

176176
a.logger.Debug("Authentication arguments added to Arc agent command")

pkg/logger/logger.go

Lines changed: 2 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import (
99
"runtime"
1010
"strings"
1111

12-
"github.com/Azure/AKSFlexNode/pkg/utils"
13-
"github.com/Azure/AKSFlexNode/pkg/utils/utilio"
1412
"github.com/sirupsen/logrus"
1513
)
1614

@@ -153,25 +151,9 @@ func setupLogFileWriter(logDir string) (io.Writer, error) {
153151

154152
logFilePath := filepath.Join(logDir, "aks-flex-node.log")
155153

156-
// Create the log file if it doesn't exist
157-
if err := createLogFileIfNotExists(logFilePath); err != nil {
158-
return nil, fmt.Errorf("failed to create log file '%s': %w", logFilePath, err)
159-
}
160-
161-
// Try to open log file for writing, handle permission issues
162-
file, err := os.OpenFile(logFilePath, os.O_WRONLY|os.O_APPEND, 0666)
154+
// Open log file for appending, creating it with 0600 if it doesn't exist
155+
file, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) //#nosec G304 - logFilePath is from trusted agent config
163156
if err != nil {
164-
// If it's a permission error and we're not running as root, try to fix permissions
165-
if os.IsPermission(err) {
166-
// Try to fix permissions using system command
167-
if fixErr := utils.RunSystemCommand("chmod", "666", logFilePath); fixErr == nil {
168-
// Retry opening the file after fixing permissions
169-
file, err = os.OpenFile(logFilePath, os.O_WRONLY|os.O_APPEND, 0666)
170-
if err == nil {
171-
return file, nil
172-
}
173-
}
174-
}
175157
return nil, fmt.Errorf("failed to open log file '%s': %w", logFilePath, err)
176158
}
177159

@@ -207,45 +189,6 @@ func setupLogFile(logger *logrus.Logger, logDir string) error {
207189
return nil
208190
}
209191

210-
// createLogFileIfNotExists creates a log file using appropriate method based on path privileges
211-
func createLogFileIfNotExists(logFilePath string) error {
212-
// Check if file already exists
213-
if utils.FileExists(logFilePath) {
214-
return nil
215-
}
216-
217-
// For systemd services, try direct file creation first since the service
218-
// should have the correct user/group and the log directory should already exist
219-
if isRunningUnderSystemd() {
220-
// Try direct file creation with appropriate permissions
221-
file, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY, 0644)
222-
if err == nil {
223-
_ = file.Close()
224-
return nil
225-
}
226-
// If direct creation fails, fall through to the system method
227-
fmt.Printf("Warning: Direct log file creation failed (%v), trying system method...\n", err)
228-
}
229-
230-
// Use WriteFileAtomicSystem to create an empty log file with proper permissions
231-
if err := utilio.WriteFile(logFilePath, []byte{}, 0644); err != nil {
232-
return err
233-
}
234-
235-
// Ensure proper ownership for the current user after file creation
236-
// Skip this for systemd services as they should already have correct ownership
237-
if !isRunningUnderSystemd() {
238-
currentUser := os.Getenv("USER")
239-
if currentUser != "" {
240-
if err := utils.RunSystemCommand("chown", currentUser+":"+currentUser, logFilePath); err != nil {
241-
fmt.Printf("Warning: Failed to change ownership of %s to %s: %v\n", logFilePath, currentUser, err)
242-
}
243-
}
244-
}
245-
246-
return nil
247-
}
248-
249192
// GetLoggerFromContext retrieves the logger from context
250193
func GetLoggerFromContext(ctx context.Context) *logrus.Logger {
251194
if logger, ok := ctx.Value(loggerContextKey).(*logrus.Logger); ok {

0 commit comments

Comments
 (0)