From e17c44ffd5c59a393ccea1d4feeb10c96c2be8d4 Mon Sep 17 00:00:00 2001 From: Julian LaNeve Date: Tue, 21 Jul 2026 14:40:29 -0400 Subject: [PATCH] Fix connection export leaving a plaintext temp file in the container The connection export wrote connection URIs, with embedded plaintext passwords, to tmp.connections inside the container, but the cleanup step tried to delete tmp.connection (missing the trailing s), so the file was never removed. This moves the export to a real path under /tmp, uses one constant for the export, read, and remove commands so the names can't drift apart again, and runs the removal via defer so a failure partway through the read or write still cleans up the file. Also fixes the success message's misspelling and grammar. Co-Authored-By: Claude Fable 5 --- settings/settings.go | 73 ++++++++++++++++++++++----------------- settings/settings_test.go | 49 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 31 deletions(-) diff --git a/settings/settings.go b/settings/settings.go index a728ce4b2..1218b2d63 100644 --- a/settings/settings.go +++ b/settings/settings.go @@ -47,11 +47,12 @@ var ( const ( airflowConnectionList = "airflow connections list -o yaml" ariflowPoolsList = "airflow pools list -o yaml" - airflowConnExport = "airflow connections export tmp.connections --file-format env" + airflowConnExport = "airflow connections export /tmp/connections.env --file-format env" airflowVarExport = "airflow variables export tmp.var" catVarFile = "cat tmp.var" rmVarFile = "rm tmp.var" - catConnFile = "cat tmp.connections" + catConnFile = "cat /tmp/connections.env" + rmConnFile = "rm /tmp/connections.env" configReadErrorMsg = "Error reading Airflow Settings file. Connections, Variables, and Pools were not loaded please check your Settings file syntax: %s\n" noColorString = "[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))" ) @@ -481,7 +482,7 @@ func EnvExportVariables(id, envFile string) error { return errors.New("variable export unsuccessful") } -func EnvExportConnections(id, envFile string) error { +func EnvExportConnections(id, envFile string) (err error) { // Airflow command to export connections to env uris out, err := execAirflowCommand(id, airflowConnExport) if err != nil { @@ -489,41 +490,51 @@ func EnvExportConnections(id, envFile string) error { } logger.Debugf("Env Export Connections logs:\n%s", out) - if strings.Contains(out, "successfully") { - // get connections from file craeted by airflow command - out, err = execAirflowCommand(id, catConnFile) - if err != nil { - return fmt.Errorf("error reading connections file: %w", err) - } + if !strings.Contains(out, "successfully") { + return errors.New("connection export unsuccessful") + } - vars := strings.Split(out, "\n") - // add connections to the env file; connection URIs contain passwords, so keep it owner-only - f, err := os.OpenFile(envFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) //nolint:mnd - if err != nil { - return errors.Wrap(err, "Writing connections to file unsuccessful") + // The command above wrote connection URIs, including plaintext passwords, to a + // temp file inside the container. Remove it once we're done reading it, even if + // something below fails, so secrets don't linger in the container. + defer func() { + if _, rmErr := execAirflowCommand(id, rmConnFile); rmErr != nil { + rmErr = fmt.Errorf("error removing connections file: %w", rmErr) + if err == nil { + err = rmErr + } else { + fmt.Println(rmErr) + } } + }() - defer f.Close() + // get connections from file created by airflow command + out, err = execAirflowCommand(id, catConnFile) + if err != nil { + return fmt.Errorf("error reading connections file: %w", err) + } - for i := range vars { - varSplit := strings.SplitN(vars[i], "=", 2) //nolint:mnd - if len(varSplit) > 1 { - fmt.Println("Exporting Connection: " + varSplit[0]) - _, err := f.WriteString("\nAIRFLOW_CONN_" + strings.ToUpper(varSplit[0]) + "=" + varSplit[1]) - if err != nil { - fmt.Printf("error adding connection %s to file: %s\n", varSplit[0], err.Error()) - } + vars := strings.Split(out, "\n") + // add connections to the env file; connection URIs contain passwords, so keep it owner-only + f, ferr := os.OpenFile(envFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) //nolint:mnd + if ferr != nil { + return errors.Wrap(ferr, "Writing connections to file unsuccessful") + } + + defer f.Close() + + for i := range vars { + varSplit := strings.SplitN(vars[i], "=", 2) //nolint:mnd + if len(varSplit) > 1 { + fmt.Println("Exporting Connection: " + varSplit[0]) + _, err := f.WriteString("\nAIRFLOW_CONN_" + strings.ToUpper(varSplit[0]) + "=" + varSplit[1]) + if err != nil { + fmt.Printf("error adding connection %s to file: %s\n", varSplit[0], err.Error()) } } - fmt.Println("Aiflow connections successfully export to the file " + envFile + "\n") - rmCmd := "rm tmp.connection" - _, err = execAirflowCommand(id, rmCmd) - if err != nil { - return fmt.Errorf("error removing connections file: %w", err) - } - return nil } - return errors.New("connection export unsuccessful") + fmt.Println("Airflow connections successfully exported to the file " + envFile + "\n") + return nil } func Export(id, settingsFile string, version uint64, connections, variables, pools bool) error { diff --git a/settings/settings_test.go b/settings/settings_test.go index 68a73f068..153abea07 100644 --- a/settings/settings_test.go +++ b/settings/settings_test.go @@ -503,6 +503,55 @@ func (s *Suite) TestEnvExport() { }) } +func (s *Suite) TestEnvExportConnectionsCleansUpTempFile() { + s.Run("export, read, and remove all target the same in-container temp path", func() { + exportedPath := strings.TrimPrefix(catConnFile, "cat ") + s.True(strings.HasPrefix(exportedPath, "/tmp/"), "connections should be exported under /tmp inside the container, got %q", exportedPath) + s.Contains(airflowConnExport, exportedPath, "export command should write to the same path the cat command reads") + s.Equal(exportedPath, strings.TrimPrefix(rmConnFile, "rm "), "remove command should target the same path the file was exported to") + }) + + s.Run("removes the temp file on success", func() { + var commands []string + execAirflowCommand = func(id, airflowCommand string) (string, error) { + commands = append(commands, airflowCommand) + switch airflowCommand { + case airflowConnExport: + return "1 connections successfully exported", nil + case catConnFile: + return "local_postgres=postgres://username:password@example.db.example.com:5432/schema", nil + default: + return "", nil + } + } + + err := EnvExportConnections("id", "testfiles/test.env") + s.NoError(err) + s.Contains(commands, rmConnFile) + _ = fileutil.WriteStringToFile("testfiles/test.env", "") + }) + + s.Run("still removes the temp file when reading the connections back fails", func() { + var commands []string + execAirflowCommand = func(id, airflowCommand string) (string, error) { + commands = append(commands, airflowCommand) + switch airflowCommand { + case airflowConnExport: + return "1 connections successfully exported", nil + case catConnFile: + return "", fmt.Errorf("boom") + default: + return "", nil + } + } + + err := EnvExportConnections("id", "testfiles/test.env") + s.Error(err) + s.Contains(err.Error(), "error reading connections file") + s.Contains(commands, rmConnFile, "the temp file should still be removed even though the read failed") + }) +} + func (s *Suite) TestExport() { s.Run("success", func() { WorkingPath = "./testfiles/"