diff --git a/backend/internal/services/gitops_sync_service.go b/backend/internal/services/gitops_sync_service.go index c95cd678b2..a48e229590 100644 --- a/backend/internal/services/gitops_sync_service.go +++ b/backend/internal/services/gitops_sync_service.go @@ -1693,6 +1693,12 @@ func (s *GitOpsSyncService) stageDirectorySyncInternal(ctx context.Context, sync return nil, fmt.Errorf("failed to get projects directory: %w", err) } + // The project-root env files are reserved for the three-file override + // merge applied below (applyGitSyncEnvToDirInternal) rather than a raw + // overwrite — a raw .env write would silently wipe edits made in Arcane + // on every sync. + filteredSyncFiles, gitEnvContent := partitionReservedRootEnvFilesInternal(ctx, syncFiles) + stagePath, err := os.MkdirTemp(projectsDir, ".gitops-sync-stage-*") if err != nil { return nil, fmt.Errorf("failed to create staging directory: %w", err) @@ -1724,12 +1730,16 @@ func (s *GitOpsSyncService) stageDirectorySyncInternal(ctx context.Context, sync return nil, err } - syncedFiles := make([]string, len(syncFiles)) - for i, file := range syncFiles { + syncedFiles := make([]string, len(filteredSyncFiles)) + for i, file := range filteredSyncFiles { syncedFiles[i] = file.RelativePath } - oldSyncedFiles := parseSyncedFiles(sync.SyncedFiles) + // Syncs created before this fix may still have .env recorded as a tracked + // file. Drop reserved root env files here too, or CleanupRemovedFiles would + // treat .env as removed-by-git and delete the live-copied stage .env before + // applyGitSyncEnvToDirInternal gets a chance to read it for the merge. + oldSyncedFiles := filterReservedRootEnvFilesInternal(parseSyncedFiles(sync.SyncedFiles)) if len(oldSyncedFiles) > 0 { if err := projects.CleanupRemovedFiles(projectsDir, stagePath, oldSyncedFiles, syncedFiles); err != nil { _ = os.RemoveAll(stagePath) @@ -1745,20 +1755,33 @@ func (s *GitOpsSyncService) stageDirectorySyncInternal(ctx context.Context, sync contentsChanged := true if project != nil { - contentsChanged, err = projects.DirectorySyncContentsChanged(project.Path, syncFiles, oldSyncedFiles, composeFileName) + contentsChanged, err = projects.DirectorySyncContentsChanged(project.Path, filteredSyncFiles, oldSyncedFiles, composeFileName) if err != nil { _ = os.RemoveAll(stagePath) return nil, fmt.Errorf("failed to compare staged directory changes: %w", err) } } - // Write the repo files after cleanup so validation sees the final on-disk - // tree exactly as it will exist in the managed project. - if _, err := projects.WriteSyncedDirectory(projectsDir, stagePath, syncFiles); err != nil { + // Write the repo files (excluding reserved root env files, handled below) + // after cleanup so validation sees the final on-disk tree exactly as it + // will exist in the managed project. + if _, err := projects.WriteSyncedDirectory(projectsDir, stagePath, filteredSyncFiles); err != nil { _ = os.RemoveAll(stagePath) return nil, fmt.Errorf("failed to write staged sync files: %w", err) } + // Route the project-root .env through the same three-file override merge + // single-file git sync uses: git is source-of-truth, edits made in Arcane + // become an override that wins, and new git-introduced keys still flow in. + preEnvContent, postEnvContent, err := s.applyGitSyncEnvToDirInternal(stagePath, projectsDir, gitEnvContent) + if err != nil { + _ = os.RemoveAll(stagePath) + return nil, err + } + if project != nil && envContentChangedInternal(preEnvContent, postEnvContent) { + contentsChanged = true + } + serviceCount, err := s.validateDirectorySyncStageInternal(ctx, sync.ProjectName, stagePath, composeFileName) if err != nil { _ = os.RemoveAll(stagePath) @@ -1776,6 +1799,94 @@ func (s *GitOpsSyncService) stageDirectorySyncInternal(ctx context.Context, sync }, nil } +// isReservedRootEnvFileInternal reports whether relPath is one of the +// project-root env bookkeeping files the override-merge system owns (.env, +// .env.git, project.env, .env.global). These are excluded from the raw +// directory-sync write and routed through applyGitSyncEnvToDirInternal +// instead. Exact-matching RelativePath intentionally leaves nested env files +// (e.g. svc/.env) in the raw write set — only the project root is +// merge-managed. +func isReservedRootEnvFileInternal(relPath string) bool { + switch relPath { + case projects.EffectiveEnvFileName, projects.GitSourceEnvFileName, projects.OverrideEnvFileName, projects.GlobalEnvFileName: + return true + default: + return false + } +} + +// filterReservedRootEnvFilesInternal drops reserved root env file paths from a +// tracked synced-file list. Syncs created before the override-merge migration +// may still have .env recorded from an earlier sync; leaving it in would make +// CleanupRemovedFiles treat .env as removed-by-git and delete the live-copied +// stage .env before applyGitSyncEnvToDirInternal can read it for the merge. +func filterReservedRootEnvFilesInternal(paths []string) []string { + filtered := make([]string, 0, len(paths)) + for _, p := range paths { + if !isReservedRootEnvFileInternal(p) { + filtered = append(filtered, p) + } + } + return filtered +} + +// partitionReservedRootEnvFilesInternal splits syncFiles into the set safe to +// write directly (everything except the reserved root env files) and the +// git-sourced root .env content, if the repo has one. A committed +// .env.git/project.env/.env.global at the project root is dropped rather than +// raw-written: those are Arcane-owned bookkeeping files that must only ever be +// produced by the override merge, never by an untrusted git payload. +func partitionReservedRootEnvFilesInternal(ctx context.Context, syncFiles []projects.SyncFile) ([]projects.SyncFile, *string) { + filtered := make([]projects.SyncFile, 0, len(syncFiles)) + var gitEnvContent *string + for _, file := range syncFiles { + if !isReservedRootEnvFileInternal(file.RelativePath) { + filtered = append(filtered, file) + continue + } + switch file.RelativePath { + case projects.EffectiveEnvFileName: + content := string(file.Content) + gitEnvContent = &content + case projects.GlobalEnvFileName: + slog.WarnContext(ctx, "dropping .env.global from git payload; project-root env files are not raw-written from git and .env.global is not project-scoped", + "path", file.RelativePath, + ) + default: + slog.WarnContext(ctx, "dropping reserved Arcane env file from git payload; will be re-derived from override merge", + "path", file.RelativePath, + ) + } + } + return filtered, gitEnvContent +} + +// applyGitSyncEnvToDirInternal routes a directory sync's project-root .env +// through the same three-file override merge single-file git sync uses +// (ProjectService.ApplyGitSyncProjectFiles), so edits made in Arcane become an +// override that wins over git while new git-introduced keys still flow in. +// dirPath already contains whatever .env/.env.git/project.env were +// copied/seeded from the live project (or a pre-existing candidate directory +// for a new project), so the existing single-file machinery works against it +// unchanged. Returns the effective .env content before and after the merge so +// the caller can decide whether the change should trigger a redeploy. +func (s *GitOpsSyncService) applyGitSyncEnvToDirInternal(dirPath, projectsDir string, gitEnvContent *string) (before, after string, err error) { + update, err := s.projectService.prepareGitSyncEnvUpdateInternal(dirPath, gitEnvContent) + if err != nil { + return "", "", fmt.Errorf("failed to resolve git env state: %w", err) + } + before = update.state.DirectContent + if update.effectiveContent != nil { + after = *update.effectiveContent + } + + if err := s.projectService.persistGitSyncEnvFilesInternal(dirPath, projectsDir, update); err != nil { + return "", "", fmt.Errorf("failed to sync git env files: %w", err) + } + + return before, after, nil +} + // seedStageEnvFromCandidateDirInternal copies env files from a pre-existing project // directory at the conventional path (projectsDir//) into the // staging directory before initial-sync validation. This lets users pre-seed @@ -1796,6 +1907,9 @@ func (s *GitOpsSyncService) seedStageEnvFromCandidateDirInternal(ctx context.Con return nil } + // A permission error (e.g. a chmod 000 or foreign-owned file) is treated + // like the file being absent: it's skipped rather than aborting the whole + // staging attempt, since seeding is a best-effort convenience. readOptional := func(name string) (string, bool, error) { content, readErr := os.ReadFile(filepath.Join(candidatePath, name)) if readErr == nil { @@ -1804,6 +1918,10 @@ func (s *GitOpsSyncService) seedStageEnvFromCandidateDirInternal(ctx context.Con if errors.Is(readErr, os.ErrNotExist) { return "", false, nil } + if errors.Is(readErr, os.ErrPermission) { + slog.WarnContext(ctx, "skipping permission-locked project env file while seeding GitOps stage", "path", filepath.Join(candidatePath, name)) + return "", false, nil + } return "", false, fmt.Errorf("read %s from %s: %w", name, candidatePath, readErr) } diff --git a/backend/internal/services/gitops_sync_service_test.go b/backend/internal/services/gitops_sync_service_test.go index f2a5d812fc..70e8878dd0 100644 --- a/backend/internal/services/gitops_sync_service_test.go +++ b/backend/internal/services/gitops_sync_service_test.go @@ -430,7 +430,9 @@ services: require.NotNil(t, project) require.True(t, created) require.True(t, changed) - require.ElementsMatch(t, []string{"docker-compose.yaml", "meta.yaml", ".env"}, syncedFiles) + // .env is a reserved root env file: it is routed through the override + // merge rather than tracked as a raw synced file. + require.ElementsMatch(t, []string{"docker-compose.yaml", "meta.yaml"}, syncedFiles) composePath, detectErr := projects.DetectComposeFile(project.Path) require.NoError(t, detectErr) @@ -542,6 +544,221 @@ services: assert.Contains(t, string(featureBytes), "worker:") } +// TestGitOpsSyncService_SyncProjectDirectory_PreservesEnvOverrideAndAddsNewGitKey +// verifies directory sync routes the project-root .env through the same +// three-file override merge single-file git sync uses: an edit made in Arcane +// (recorded as project.env) survives the sync, while a new key introduced by +// git still flows into the effective .env. This is the core regression test +// for https://github.com/getarcaneapp/arcane/issues/2476. +func TestGitOpsSyncService_SyncProjectDirectory_PreservesEnvOverrideAndAddsNewGitKey(t *testing.T) { + ctx := context.Background() + svc, db, projectsDir := setupGitOpsSyncDirectoryTestService(t) + + projectPath := filepath.Join(projectsDir, "demo-project") + require.NoError(t, os.MkdirAll(projectPath, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, "docker-compose.yaml"), []byte(`services: + app: + image: nginx:1.26-alpine +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, ".env.git"), []byte("FOO=git\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, "project.env"), []byte("FOO=useredit\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, ".env"), []byte("FOO=useredit\n"), 0o644)) + + project := &models.Project{ + BaseModel: models.BaseModel{ID: "proj-directory-env-preserve"}, + Name: "demo-project", + DirName: new("demo-project"), + Path: projectPath, + Status: models.ProjectStatusStopped, + } + require.NoError(t, db.Create(project).Error) + + oldSyncedFilesJSON, err := json.Marshal([]string{"docker-compose.yaml"}) + require.NoError(t, err) + + sync := &models.GitOpsSync{ + BaseModel: models.BaseModel{ID: "sync-directory-env-preserve"}, + Name: "demo-sync", + EnvironmentID: "0", + RepositoryID: "repo-1", + ComposePath: "apps/demo/docker-compose.yaml", + ProjectName: "demo-project", + ProjectID: &project.ID, + SyncDirectory: true, + SyncedFiles: new(string(oldSyncedFilesJSON)), + } + require.NoError(t, db.Create(sync).Error) + + syncFiles := []projects.SyncFile{ + { + RelativePath: "docker-compose.yaml", + Content: []byte(`services: + app: + image: nginx:1.27-alpine +`), + }, + { + RelativePath: ".env", + Content: []byte("FOO=gitnew\nBAR=new\n"), + }, + } + + updatedProject, syncedFiles, created, changed, err := svc.syncProjectDirectoryInternal(ctx, sync, syncFiles, models.User{}) + require.NoError(t, err) + require.NotNil(t, updatedProject) + require.False(t, created) + require.True(t, changed) + require.ElementsMatch(t, []string{"docker-compose.yaml"}, syncedFiles) + + effectiveEnv, err := projects.ParseProjectEnvFile(filepath.Join(updatedProject.Path, ".env"), nil) + require.NoError(t, err) + assert.Equal(t, projects.EnvMap{"FOO": "useredit", "BAR": "new"}, effectiveEnv) + + gitEnv, err := projects.ParseProjectEnvFile(filepath.Join(updatedProject.Path, ".env.git"), nil) + require.NoError(t, err) + assert.Equal(t, projects.EnvMap{"FOO": "gitnew", "BAR": "new"}, gitEnv) + + overrideEnv, err := projects.ParseProjectEnvFile(filepath.Join(updatedProject.Path, "project.env"), nil) + require.NoError(t, err) + assert.Equal(t, projects.EnvMap{"FOO": "useredit"}, overrideEnv) +} + +// TestGitOpsSyncService_SyncProjectDirectory_MigratesLegacyTrackedEnvOnFirstSyncAfterUpgrade +// verifies the first directory sync after upgrading from a pre-override-merge +// Arcane version: sync.SyncedFiles still lists ".env" from the old raw-write +// path, and the project has only a direct .env (no .env.git/project.env yet). +// CleanupRemovedFiles must not delete the live-copied stage .env before the +// merge step reads it, or a local-only key would be silently dropped instead +// of migrated into project.env. +func TestGitOpsSyncService_SyncProjectDirectory_MigratesLegacyTrackedEnvOnFirstSyncAfterUpgrade(t *testing.T) { + ctx := context.Background() + svc, db, projectsDir := setupGitOpsSyncDirectoryTestService(t) + + projectPath := filepath.Join(projectsDir, "demo-project") + require.NoError(t, os.MkdirAll(projectPath, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, "docker-compose.yaml"), []byte(`services: + app: + image: nginx:1.26-alpine +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, ".env"), []byte("LOCAL_ONLY=1\n"), 0o644)) + + project := &models.Project{ + BaseModel: models.BaseModel{ID: "proj-directory-legacy-env-migrate"}, + Name: "demo-project", + DirName: new("demo-project"), + Path: projectPath, + Status: models.ProjectStatusStopped, + } + require.NoError(t, db.Create(project).Error) + + // A pre-fix sync recorded .env as a plain tracked file. + oldSyncedFilesJSON, err := json.Marshal([]string{"docker-compose.yaml", ".env"}) + require.NoError(t, err) + + sync := &models.GitOpsSync{ + BaseModel: models.BaseModel{ID: "sync-directory-legacy-env-migrate"}, + Name: "demo-sync", + EnvironmentID: "0", + RepositoryID: "repo-1", + ComposePath: "apps/demo/docker-compose.yaml", + ProjectName: "demo-project", + ProjectID: &project.ID, + SyncDirectory: true, + SyncedFiles: new(string(oldSyncedFilesJSON)), + } + require.NoError(t, db.Create(sync).Error) + + syncFiles := []projects.SyncFile{ + { + RelativePath: "docker-compose.yaml", + Content: []byte(`services: + app: + image: nginx:1.27-alpine +`), + }, + { + RelativePath: ".env", + Content: []byte("BASE=git\n"), + }, + } + + updatedProject, _, created, _, err := svc.syncProjectDirectoryInternal(ctx, sync, syncFiles, models.User{}) + require.NoError(t, err) + require.NotNil(t, updatedProject) + require.False(t, created) + + effectiveEnv, err := projects.ParseProjectEnvFile(filepath.Join(updatedProject.Path, ".env"), nil) + require.NoError(t, err) + assert.Equal(t, projects.EnvMap{"BASE": "git", "LOCAL_ONLY": "1"}, effectiveEnv) + + overrideEnv, err := projects.ParseProjectEnvFile(filepath.Join(updatedProject.Path, "project.env"), nil) + require.NoError(t, err) + assert.Equal(t, projects.EnvMap{"LOCAL_ONLY": "1"}, overrideEnv) +} + +// TestGitOpsSyncService_SyncProjectDirectory_IgnoresCommittedReservedEnvFiles verifies +// that a repo committing files at the reserved bookkeeping paths (.env.git, +// project.env) cannot clobber Arcane's own override-merge bookkeeping: those paths +// are dropped from the raw sync write, never tracked in syncedFiles, and the actual +// .env.git/project.env on disk are still produced solely by the merge. +func TestGitOpsSyncService_SyncProjectDirectory_IgnoresCommittedReservedEnvFiles(t *testing.T) { + ctx := context.Background() + svc, db, _ := setupGitOpsSyncDirectoryTestService(t) + + sync := &models.GitOpsSync{ + BaseModel: models.BaseModel{ID: "sync-directory-reserved-env"}, + Name: "demo-sync", + EnvironmentID: "0", + RepositoryID: "repo-1", + ComposePath: "apps/demo/docker-compose.yaml", + ProjectName: "demo-project-reserved", + SyncDirectory: true, + } + require.NoError(t, db.Create(sync).Error) + + syncFiles := []projects.SyncFile{ + { + RelativePath: "docker-compose.yaml", + Content: []byte(`services: + app: + image: nginx:alpine +`), + }, + { + RelativePath: ".env", + Content: []byte("FOO=fromgit\n"), + }, + { + RelativePath: ".env.git", + Content: []byte("poison\n"), + }, + { + RelativePath: "project.env", + Content: []byte("poison\n"), + }, + } + + project, syncedFiles, created, _, err := svc.syncProjectDirectoryInternal(ctx, sync, syncFiles, models.User{}) + require.NoError(t, err) + require.NotNil(t, project) + require.True(t, created) + require.ElementsMatch(t, []string{"docker-compose.yaml"}, syncedFiles) + + effectiveEnv, err := projects.ParseProjectEnvFile(filepath.Join(project.Path, ".env"), nil) + require.NoError(t, err) + assert.Equal(t, projects.EnvMap{"FOO": "fromgit"}, effectiveEnv) + + gitBytes, err := os.ReadFile(filepath.Join(project.Path, ".env.git")) + require.NoError(t, err) + assert.NotContains(t, string(gitBytes), "poison") + gitEnv, err := projects.ParseProjectEnvFile(filepath.Join(project.Path, ".env.git"), nil) + require.NoError(t, err) + assert.Equal(t, projects.EnvMap{"FOO": "fromgit"}, gitEnv) + + _, statErr := os.Stat(filepath.Join(project.Path, "project.env")) + assert.True(t, os.IsNotExist(statErr)) +} + func TestGitOpsSyncService_DirectorySync_RealWalkWithNestedConfig(t *testing.T) { ctx := context.Background() svc, db, _ := setupGitOpsSyncDirectoryTestService(t) diff --git a/backend/internal/services/project_service.go b/backend/internal/services/project_service.go index 513ac5f043..5fcbc232f0 100644 --- a/backend/internal/services/project_service.go +++ b/backend/internal/services/project_service.go @@ -2279,7 +2279,10 @@ func (s *ProjectService) createProjectInternal(ctx context.Context, name, compos return nil, wrapProjectFileErrorInternal(err) } - if err := s.validateComposeContentForUpdate(ctx, projectsDirectory, projectPath, name, composeContent, envContent); err != nil { + // GitOps-originated creates (allowNameSuffix=false) tolerate not-yet-supplied + // ${VAR} references the same way single-file git sync updates do; interactive + // creates (allowNameSuffix=true) stay strict. + if err := s.validateComposeContentForUpdate(ctx, projectsDirectory, projectPath, name, composeContent, envContent, !allowNameSuffix); err != nil { _ = os.RemoveAll(projectPath) return nil, fmt.Errorf("invalid compose file: %w", err) } @@ -3268,7 +3271,7 @@ func (s *ProjectService) ApplyGitSyncProjectFiles(ctx context.Context, projectID return nil, fmt.Errorf("failed to resolve git env state: %w", err) } - if err := s.validateComposeContentForUpdate(ctx, projectsDirectory, proj.Path, proj.Name, composeContent, envUpdate.effectiveContent); err != nil { + if err := s.validateComposeContentForUpdate(ctx, projectsDirectory, proj.Path, proj.Name, composeContent, envUpdate.effectiveContent, true); err != nil { return nil, fmt.Errorf("invalid compose file: %w", err) } @@ -3571,7 +3574,7 @@ func (s *ProjectService) persistUpdatedProjectFiles(ctx context.Context, proj *m if err != nil { return fmt.Errorf("invalid compose file: %w", err) } - if err := s.validateComposeContentForUpdate(ctx, projectsDirectory, proj.Path, proj.Name, *composeContent, effectiveEnvContent); err != nil { + if err := s.validateComposeContentForUpdate(ctx, projectsDirectory, proj.Path, proj.Name, *composeContent, effectiveEnvContent, false); err != nil { return fmt.Errorf("invalid compose file: %w", err) } if err := projects.WriteComposeFile(projectsDirectory, proj.Path, *composeContent); err != nil { @@ -3593,7 +3596,7 @@ func (s *ProjectService) persistUpdatedProjectFiles(ctx context.Context, proj *m return nil } -func (s *ProjectService) validateComposeContentForUpdate(ctx context.Context, projectsDirectory, projectPath, projectName, composeContent string, effectiveEnvContent *string) (err error) { +func (s *ProjectService) validateComposeContentForUpdate(ctx context.Context, projectsDirectory, projectPath, projectName, composeContent string, effectiveEnvContent *string, lenient bool) (err error) { defer func() { if recovered := recover(); recovered != nil { err = fmt.Errorf("compose file contains invalid syntax: %v", recovered) @@ -3628,6 +3631,9 @@ func (s *ProjectService) validateComposeContentForUpdate(ctx context.Context, pr if validationProjectName != "" { opts.SetProjectName(validationProjectName, true) } + if lenient { + projects.ApplyLenientLoaderOptions(ctx, opts, cfg.ConfigFiles[0].Filename) + } }) return loadErr }) @@ -3792,7 +3798,18 @@ func withTransientValidationEnvFile(projectPath string, effectiveEnvContent *str originalContent, readErr := os.ReadFile(envPath) originalExists := readErr == nil if readErr != nil && !os.IsNotExist(readErr) { - return fmt.Errorf("prepare env file for compose validation: %w", readErr) + if !errors.Is(readErr, os.ErrPermission) { + return fmt.Errorf("prepare env file for compose validation: %w", readErr) + } + // The file exists but is permission-locked (e.g. chmod 000, foreign-owned). + // Its contents can't be verified or safely overwritten, so leave it + // untouched and validate against whatever's already on disk instead of + // aborting the whole update. + slog.Warn("skipping permission-locked .env file during compose validation; leaving it untouched", "projectPath", projectPath) + if run == nil { + return nil + } + return run() } shouldWrite := effectiveEnvContent != nil || !originalExists @@ -3878,7 +3895,7 @@ func (s *ProjectService) persistEffectiveEnvContentInternal(projectPath, project return err } } - return projects.WriteEnvFile(projectsDirectory, projectPath, envContent) + return projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.EffectiveEnvFileName, state.EffectiveUnreadable, envContent) } overrideContent, err := projects.BuildOverrideEnvContent(state.GitContent, envContent) @@ -3891,19 +3908,11 @@ func (s *ProjectService) persistEffectiveEnvContentInternal(projectPath, project return fmt.Errorf("build effective env content: %w", err) } - if err := projects.WriteEnvFile(projectsDirectory, projectPath, effectiveContent); err != nil { + if err := projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.EffectiveEnvFileName, state.EffectiveUnreadable, effectiveContent); err != nil { return err } - if strings.TrimSpace(overrideContent) == "" { - if err := projects.RemoveProjectFile(projectsDirectory, projectPath, projects.OverrideEnvFileName); err != nil { - return err - } - } else if err := projects.WriteProjectFile(projectsDirectory, projectPath, projects.OverrideEnvFileName, overrideContent); err != nil { - return err - } - - return nil + return projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.OverrideEnvFileName, state.OverrideUnreadable, overrideContent) } func (s *ProjectService) ensureEffectiveEnvFileInternal(projectPath, projectsDirectory string) error { @@ -3921,7 +3930,7 @@ func (s *ProjectService) ensureEffectiveEnvFileInternal(projectPath, projectsDir if err != nil { return err } - return projects.WriteEnvFile(projectsDirectory, projectPath, effectiveContent) + return projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.EffectiveEnvFileName, state.EffectiveUnreadable, effectiveContent) } return projects.EnsureEnvFile(projectsDirectory, projectPath) } @@ -3931,7 +3940,7 @@ func (s *ProjectService) ensureEffectiveEnvFileInternal(projectPath, projectsDir return fmt.Errorf("build effective env content: %w", err) } - return projects.WriteEnvFile(projectsDirectory, projectPath, effectiveContent) + return projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.EffectiveEnvFileName, state.EffectiveUnreadable, effectiveContent) } type gitSyncEnvUpdateInternal struct { @@ -4025,7 +4034,11 @@ func (s *ProjectService) persistGitSyncEnvFilesInternal(projectPath, projectsDir if update.effectiveContent != nil { effectiveContent = *update.effectiveContent } - return projects.WriteEnvFile(projectsDirectory, projectPath, effectiveContent) + return projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.EffectiveEnvFileName, update.state.EffectiveUnreadable, effectiveContent) + } + if update.state.EffectiveUnreadable { + slog.Warn("skipping permission-locked .env file; leaving it untouched", "projectPath", projectPath) + return nil } return projects.EnsureEnvFile(projectsDirectory, projectPath) } @@ -4034,21 +4047,13 @@ func (s *ProjectService) persistGitSyncEnvFilesInternal(projectPath, projectsDir return errors.New("missing effective env content for git sync update") } - if err := projects.WriteEnvFile(projectsDirectory, projectPath, *update.effectiveContent); err != nil { - return err - } - if err := projects.WriteProjectFile(projectsDirectory, projectPath, projects.GitSourceEnvFileName, *update.gitEnvContent); err != nil { + if err := projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.EffectiveEnvFileName, update.state.EffectiveUnreadable, *update.effectiveContent); err != nil { return err } - if strings.TrimSpace(update.overrideContent) == "" { - if err := projects.RemoveProjectFile(projectsDirectory, projectPath, projects.OverrideEnvFileName); err != nil { - return err - } - } else if err := projects.WriteProjectFile(projectsDirectory, projectPath, projects.OverrideEnvFileName, update.overrideContent); err != nil { + if err := projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.GitSourceEnvFileName, update.state.GitSourceUnreadable, *update.gitEnvContent); err != nil { return err } - - return nil + return projects.WriteManagedEnvFile(projectsDirectory, projectPath, projects.OverrideEnvFileName, update.state.OverrideUnreadable, update.overrideContent) } func (s *ProjectService) ensureProjectStoppedForRenameInternal(ctx context.Context, proj *models.Project, name *string) error { diff --git a/backend/internal/services/project_service_test.go b/backend/internal/services/project_service_test.go index f2ff5014db..41ef2ab73e 100644 --- a/backend/internal/services/project_service_test.go +++ b/backend/internal/services/project_service_test.go @@ -3162,6 +3162,58 @@ func TestProjectService_ApplyGitSyncProjectFiles_UsesGlobalEnvDuringComposeValid assert.Equal(t, compose, string(composeBytes)) } +// TestProjectService_ApplyGitSyncProjectFiles_TolerantOfUndefinedComposeVar verifies +// a git sync updating a compose file that references an undefined ${VAR} (with no +// .env supplying it yet) succeeds instead of failing compose validation with a +// typed-decode error like `strconv.ParseFloat: parsing "": invalid syntax`. +func TestProjectService_ApplyGitSyncProjectFiles_TolerantOfUndefinedComposeVar(t *testing.T) { + db := setupProjectTestDB(t) + ctx := context.Background() + + projectsDir := t.TempDir() + t.Setenv("PROJECTS_DIRECTORY", projectsDir) + + settingsService, err := NewSettingsService(ctx, db) + require.NoError(t, err) + + eventService := NewEventService(db, nil, nil) + svc := NewProjectService(db, settingsService, eventService, nil, nil, nil, nil, config.Load()) + + dirName := "git-sync-undefined-var" + projectPath := filepath.Join(projectsDir, dirName) + require.NoError(t, os.MkdirAll(projectPath, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, "compose.yaml"), []byte("services:\n app:\n image: nginx:alpine\n"), 0o600)) + + compose := `services: + app: + image: nginx:alpine + deploy: + resources: + limits: + cpus: "${CPU}" +` + + project := &models.Project{ + BaseModel: models.BaseModel{ID: "proj-git-sync-undefined-var"}, + Name: dirName, + DirName: &dirName, + Path: projectPath, + Status: models.ProjectStatusStopped, + } + require.NoError(t, db.Create(project).Error) + + updated, err := svc.ApplyGitSyncProjectFiles(ctx, project.ID, compose, nil, models.User{ + BaseModel: models.BaseModel{ID: "u1"}, + Username: "tester", + }) + require.NoError(t, err) + require.NotNil(t, updated) + + composeBytes, readErr := os.ReadFile(filepath.Join(projectPath, "compose.yaml")) + require.NoError(t, readErr) + assert.Equal(t, compose, string(composeBytes)) +} + func TestProjectService_PersistGitSyncEnvFiles_UsesPreparedState(t *testing.T) { db := setupProjectTestDB(t) ctx := context.Background() diff --git a/backend/internal/services/project_service_unix_test.go b/backend/internal/services/project_service_unix_test.go new file mode 100644 index 0000000000..998b33bbc3 --- /dev/null +++ b/backend/internal/services/project_service_unix_test.go @@ -0,0 +1,73 @@ +//go:build unix + +package services + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/getarcaneapp/arcane/backend/v2/internal/config" + "github.com/getarcaneapp/arcane/backend/v2/internal/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestProjectService_ApplyGitSyncProjectFiles_TolerantOfPermissionLockedEnv verifies +// a permission-locked (e.g. chmod 000, foreign-owned) .env does not brick a git sync: +// the compose file still updates, the sync succeeds, and the locked file is left +// untouched instead of aborting the whole update. +func TestProjectService_ApplyGitSyncProjectFiles_TolerantOfPermissionLockedEnv(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("permission bits are ignored when running as root") + } + + db := setupProjectTestDB(t) + ctx := context.Background() + + projectsDir := t.TempDir() + t.Setenv("PROJECTS_DIRECTORY", projectsDir) + + settingsService, err := NewSettingsService(ctx, db) + require.NoError(t, err) + + eventService := NewEventService(db, nil, nil) + svc := NewProjectService(db, settingsService, eventService, nil, nil, nil, nil, config.Load()) + + dirName := "git-sync-locked-env" + projectPath := filepath.Join(projectsDir, dirName) + require.NoError(t, os.MkdirAll(projectPath, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectPath, "compose.yaml"), []byte("services:\n app:\n image: nginx:alpine\n"), 0o600)) + + envPath := filepath.Join(projectPath, ".env") + require.NoError(t, os.WriteFile(envPath, []byte("FOO=locked\n"), 0o600)) + require.NoError(t, os.Chmod(envPath, 0o000)) + t.Cleanup(func() { _ = os.Chmod(envPath, 0o644) }) + + project := &models.Project{ + BaseModel: models.BaseModel{ID: "proj-git-sync-locked-env"}, + Name: "git-sync-locked-env", + DirName: &dirName, + Path: projectPath, + Status: models.ProjectStatusStopped, + } + require.NoError(t, db.Create(project).Error) + + updated, err := svc.ApplyGitSyncProjectFiles(ctx, project.ID, "services:\n app:\n image: nginx:1.27-alpine\n", new("FOO=fromgit\n"), models.User{ + BaseModel: models.BaseModel{ID: "u1"}, + Username: "tester", + }) + require.NoError(t, err) + require.NotNil(t, updated) + + composeBytes, err := os.ReadFile(filepath.Join(projectPath, "compose.yaml")) + require.NoError(t, err) + assert.Contains(t, string(composeBytes), "nginx:1.27-alpine") + + // The locked .env survived untouched — sync did not attempt to overwrite it. + require.NoError(t, os.Chmod(envPath, 0o644)) + envBytes, err := os.ReadFile(envPath) + require.NoError(t, err) + assert.Equal(t, "FOO=locked\n", string(envBytes)) +} diff --git a/backend/pkg/projects/env.go b/backend/pkg/projects/env.go index 62e64d657a..55946142bb 100644 --- a/backend/pkg/projects/env.go +++ b/backend/pkg/projects/env.go @@ -48,6 +48,15 @@ type ProjectEnvState struct { HasGitSource bool OverrideContent string HasOverride bool + // The *Unreadable fields report a file that exists on disk but could not be + // read because of a permission error (e.g. a chmod 000 or foreign-owned + // file reachable through a bind mount). Such a file is treated as absent + // for merge purposes, and callers persisting env state must not attempt to + // write or remove it — its contents are unknown, so writing could either + // fail (bricking the caller) or silently clobber operator intent. + EffectiveUnreadable bool + GitSourceUnreadable bool + OverrideUnreadable bool } type EnvLoader struct { @@ -335,29 +344,41 @@ func buildOverrideEnvContentInternal(gitContent, effectiveContent string) (strin } func ReadProjectEnvState(projectPath string) (ProjectEnvState, error) { - effectiveContent, hasEffective, err := readOptionalProjectFileInternal(projectPath, EffectiveEnvFileName) + effectiveContent, hasEffective, effectiveUnreadable, err := readOptionalProjectFileInternal(projectPath, EffectiveEnvFileName) if err != nil { return ProjectEnvState{}, err } - gitContent, hasGitSource, err := readOptionalProjectFileInternal(projectPath, GitSourceEnvFileName) + gitContent, hasGitSource, gitSourceUnreadable, err := readOptionalProjectFileInternal(projectPath, GitSourceEnvFileName) if err != nil { return ProjectEnvState{}, err } - overrideContent, hasOverride, err := readOptionalProjectFileInternal(projectPath, OverrideEnvFileName) + overrideContent, hasOverride, overrideUnreadable, err := readOptionalProjectFileInternal(projectPath, OverrideEnvFileName) if err != nil { return ProjectEnvState{}, err } + if effectiveUnreadable || gitSourceUnreadable || overrideUnreadable { + slog.Warn("skipping permission-locked project env file(s); leaving them untouched", + "projectPath", projectPath, + "effectiveUnreadable", effectiveUnreadable, + "gitSourceUnreadable", gitSourceUnreadable, + "overrideUnreadable", overrideUnreadable, + ) + } + state := ProjectEnvState{ - DirectContent: effectiveContent, - EffectiveContent: effectiveContent, - HasEffective: hasEffective, - GitContent: gitContent, - HasGitSource: hasGitSource, - OverrideContent: overrideContent, - HasOverride: hasOverride, + DirectContent: effectiveContent, + EffectiveContent: effectiveContent, + HasEffective: hasEffective, + EffectiveUnreadable: effectiveUnreadable, + GitContent: gitContent, + HasGitSource: hasGitSource, + GitSourceUnreadable: gitSourceUnreadable, + OverrideContent: overrideContent, + HasOverride: hasOverride, + OverrideUnreadable: overrideUnreadable, } if hasGitSource || hasOverride { @@ -383,6 +404,33 @@ func ReadProjectEnvState(projectPath string) (ProjectEnvState, error) { return state, nil } +// WriteManagedEnvFile writes (or, for project.env, removes) one of the three +// env-merge bookkeeping files — fileName must be EffectiveEnvFileName, +// GitSourceEnvFileName, or OverrideEnvFileName. If the existing file is +// permission-locked, the write is skipped and a warning logged instead: its +// contents can't be verified, and a locked file is typically unwritable too, +// so attempting the write would abort the whole caller. +func WriteManagedEnvFile(projectsDirectory, projectPath, fileName string, unreadable bool, content string) error { + if unreadable { + slog.Warn("skipping permission-locked project env file; leaving it untouched", "projectPath", projectPath, "file", fileName) + return nil + } + + switch fileName { + case EffectiveEnvFileName: + return WriteEnvFile(projectsDirectory, projectPath, content) + case GitSourceEnvFileName: + return WriteProjectFile(projectsDirectory, projectPath, GitSourceEnvFileName, content) + case OverrideEnvFileName: + if strings.TrimSpace(content) == "" { + return RemoveProjectFile(projectsDirectory, projectPath, OverrideEnvFileName) + } + return WriteProjectFile(projectsDirectory, projectPath, OverrideEnvFileName, content) + default: + return fmt.Errorf("write managed env file: unsupported file name %q", fileName) + } +} + // parseEnvWithContext parses environment variables from an io.Reader using compose-go's // dotenv parser with variable expansion using the provided context lookup map. func parseEnvWithContext(r io.Reader, contextEnv EnvMap) (EnvMap, error) { @@ -404,15 +452,24 @@ func parseEnvWithContext(r io.Reader, contextEnv EnvMap) (EnvMap, error) { return envMap, nil } -func readOptionalProjectFileInternal(projectPath, fileName string) (string, bool, error) { - content, err := os.ReadFile(filepath.Join(projectPath, fileName)) - if err == nil { - return string(content), true, nil +// readOptionalProjectFileInternal reads fileName from projectPath. A missing +// file is reported via exists=false with no error. A permission error is +// reported via unreadable=true with no error: the file is present but its +// contents cannot be verified, so callers must treat it as absent for merge +// purposes and must not attempt to overwrite or remove it. Any other I/O +// error (e.g. the path is a directory) is still returned as a hard failure. +func readOptionalProjectFileInternal(projectPath, fileName string) (content string, exists, unreadable bool, err error) { + raw, readErr := os.ReadFile(filepath.Join(projectPath, fileName)) + if readErr == nil { + return string(raw), true, false, nil + } + if errors.Is(readErr, os.ErrNotExist) { + return "", false, false, nil } - if errors.Is(err, os.ErrNotExist) { - return "", false, nil + if errors.Is(readErr, os.ErrPermission) { + return "", false, true, nil } - return "", false, fmt.Errorf("read %s: %w", fileName, err) + return "", false, false, fmt.Errorf("read %s: %w", fileName, readErr) } // formatEnvMapInternal serializes env maps into Arcane's canonical generated diff --git a/backend/pkg/projects/env_unix_test.go b/backend/pkg/projects/env_unix_test.go new file mode 100644 index 0000000000..783f21baf2 --- /dev/null +++ b/backend/pkg/projects/env_unix_test.go @@ -0,0 +1,41 @@ +//go:build unix + +package projects + +import ( + "os" + "path/filepath" + "testing" + + pkgutils "github.com/getarcaneapp/arcane/backend/v2/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestReadProjectEnvState_TreatsPermissionLockedEnvAsUnreadable verifies that a +// chmod 000 .env (e.g. root-owned or foreign-owned) is reported as present but +// unreadable instead of failing the whole read, so callers can leave it +// untouched rather than aborting a git sync. +func TestReadProjectEnvState_TreatsPermissionLockedEnvAsUnreadable(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("permission bits are ignored when running as root") + } + + projectDir := t.TempDir() + envPath := filepath.Join(projectDir, EffectiveEnvFileName) + require.NoError(t, os.WriteFile(envPath, []byte("FOO=locked\n"), pkgutils.FilePerm)) + require.NoError(t, os.Chmod(envPath, 0o000)) + t.Cleanup(func() { _ = os.Chmod(envPath, 0o644) }) + + state, err := ReadProjectEnvState(projectDir) + require.NoError(t, err) + assert.True(t, state.EffectiveUnreadable) + assert.False(t, state.HasEffective) + assert.Empty(t, state.EffectiveContent) + + // The file itself is left untouched. + require.NoError(t, os.Chmod(envPath, 0o644)) + content, err := os.ReadFile(envPath) + require.NoError(t, err) + assert.Equal(t, "FOO=locked\n", string(content)) +} diff --git a/backend/pkg/projects/load.go b/backend/pkg/projects/load.go index ff9005910b..898df7f72d 100644 --- a/backend/pkg/projects/load.go +++ b/backend/pkg/projects/load.go @@ -213,6 +213,34 @@ func lenientCastSizeInternal(value string) (any, error) { return int(n), nil } +// ApplyLenientLoaderOptions configures a compose loader.Options for tolerant +// loading: structural validation and consistency checks are skipped, and +// undefined ${VAR} references are substituted with a placeholder instead of +// an empty string (which would otherwise produce invalid volume/bind specs +// like ":/path"). Callers use this when a .env file may not yet exist or may +// not define every variable the compose file references. +func ApplyLenientLoaderOptions(ctx context.Context, opts *loader.Options, composeFile string) { + opts.SkipValidation = true + opts.SkipConsistencyCheck = true + if opts.Interpolate == nil { + slog.WarnContext(ctx, "compose loader did not initialize Interpolate options; lenient variable substitution will not apply", "compose_file", composeFile) + return + } + + realLookup := opts.Interpolate.LookupValue + opts.Interpolate = &interp.Options{ + Substitute: template.Substitute, + TypeCastMapping: wrapTypeCastMappingLenientInternal(opts.Interpolate.TypeCastMapping), + LookupValue: func(key string) (string, bool) { + if val, ok := realLookup(key); ok { + return val, true + } + slog.DebugContext(ctx, "compose variable undefined during lenient load, using placeholder", "variable", key, "compose_file", composeFile) + return "/placeholder-undefined", true + }, + } +} + func loadComposeProjectInternal( ctx context.Context, composeFile string, @@ -274,24 +302,7 @@ func loadComposeProjectInternal( // does, so config-hashes match and both tools stop recreating services. loader.WithDiscardEnvFiles(opts) if lenient { - opts.SkipValidation = true - opts.SkipConsistencyCheck = true - if opts.Interpolate == nil { - slog.WarnContext(ctx, "compose loader did not initialize Interpolate options; lenient variable substitution will not apply", "compose_file", composeFile) - } else { - realLookup := opts.Interpolate.LookupValue - opts.Interpolate = &interp.Options{ - Substitute: template.Substitute, - TypeCastMapping: wrapTypeCastMappingLenientInternal(opts.Interpolate.TypeCastMapping), - LookupValue: func(key string) (string, bool) { - if val, ok := realLookup(key); ok { - return val, true - } - slog.DebugContext(ctx, "compose variable undefined during lenient load, using placeholder", "variable", key, "compose_file", composeFile) - return "/placeholder-undefined", true - }, - } - } + ApplyLenientLoaderOptions(ctx, opts, composeFile) } if configureLoader != nil { configureLoader(opts)