diff --git a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift index 36131478..cb1f2419 100644 --- a/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift +++ b/supacode/Domain/AgentProfile/AgentProfileLaunchPlan.swift @@ -10,8 +10,10 @@ nonisolated struct AgentProfileLaunchPlan: Equatable, Sendable { let invocation: AgentInvocation let placement: AgentProfilePlacement let splitDirection: UserCustomSplitDirection - /// Environment patch for the new surface. Non-empty only for account-bound - /// profiles; additive over the shell's normal environment, never a scrub. + /// Environment patch for the new surface: the profile's user overrides + /// plus, for account-bound profiles, the dedicated-home variable. Applied at + /// surface spawn, so the shell and everything started in it see it; additive + /// over the shell's normal environment, never a scrub. let environment: [String: String] /// Dedicated home to provision before launch; nil for pure presets. let dedicatedHome: URL? @@ -72,8 +74,12 @@ nonisolated enum AgentProfileEnvironmentPolicy { } } + /// `HOME` is reserved for the same reason as the account-home variables: + /// relocating it moves every runtime's *default* home (`$HOME/.claude`, + /// `$HOME/.codex`), bypassing home provisioning, deletion protection, and + /// rooted session detection without ever flipping the binding toggle. static func isReserved(_ name: String) -> Bool { - name.hasPrefix("PROWL_") || reservedNames.contains(name) + name.hasPrefix("PROWL_") || name == "HOME" || reservedNames.contains(name) } /// A NUL would be silently truncated at the C-string boundary; refuse the diff --git a/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift b/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift index 0bc98275..8ce97b7e 100644 --- a/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift +++ b/supacode/Features/Settings/BusinessLogic/RepositoryLocalSettingsPersistence.swift @@ -9,7 +9,13 @@ nonisolated struct RepositoryLocalSettingsStorage: Sendable { nonisolated enum RepositoryLocalSettingsStorageKey: DependencyKey { static var liveValue: RepositoryLocalSettingsStorage { RepositoryLocalSettingsStorage( - load: { try Data(contentsOf: $0) }, + load: { url in + let data = try Data(contentsOf: url) + // Files written before saves enforced 0600 migrate on first read + // (docs-ai 053/005), not on their next save. + SymlinkPreservingFileWriter.restrictToOwnerOnly(url) + return data + }, // Per-repo settings live under `~/.prowl/repo//` (not inside the // cloned repo), so they are user-owned config a dotfiles user may symlink // — follow the link on write to preserve it (#478). Upstream keeps a diff --git a/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift b/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift index 262faccd..d1edadbc 100644 --- a/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift +++ b/supacode/Features/Settings/BusinessLogic/SettingsFilePersistence.swift @@ -10,7 +10,13 @@ nonisolated struct SettingsFileStorage: Sendable { nonisolated enum SettingsFileStorageKey: DependencyKey { static var liveValue: SettingsFileStorage { SettingsFileStorage( - load: { try Data(contentsOf: $0) }, + load: { url in + let data = try Data(contentsOf: url) + // Files written before saves enforced 0600 migrate on first read + // (docs-ai 053/005), not on their next save. + SymlinkPreservingFileWriter.restrictToOwnerOnly(url) + return data + }, // Follows a symlinked destination to its real file so a user who links // `~/.prowl/settings.json` (and the repository-entries / appearances // files that share this closure) into a dotfiles repo keeps the link diff --git a/supacode/Features/Settings/BusinessLogic/SymlinkPreservingFileWriter.swift b/supacode/Features/Settings/BusinessLogic/SymlinkPreservingFileWriter.swift index edc2f5e2..19ccc918 100644 --- a/supacode/Features/Settings/BusinessLogic/SymlinkPreservingFileWriter.swift +++ b/supacode/Features/Settings/BusinessLogic/SymlinkPreservingFileWriter.swift @@ -7,6 +7,11 @@ nonisolated enum SymlinkPreservingFileWriterError: Error, Equatable { /// The chain exceeds the kernel's symlink-resolution limit, so the loader /// could never follow it; refuse rather than write a file it can't read back. case symbolicLinkChainTooDeep(URL) + /// The owner-only temporary file could not be created in the target's + /// directory, so there is nothing safe to rename into place. + case temporaryFileCreationFailed(URL) + /// `rename(2)` of the temporary file onto the target failed with this errno. + case renameFailed(URL, code: Int32) } /// Atomic file writes that survive a symlinked destination. When the target is a @@ -21,20 +26,50 @@ nonisolated enum SymlinkPreservingFileWriter { /// the write rather than fabricating a phantom tree there). static func write(_ data: Data, to url: URL) throws { let target = try resolvedTarget(for: url) - try FileManager.default.createDirectory( + let fileManager = FileManager.default + try fileManager.createDirectory( at: url.deletingLastPathComponent(), withIntermediateDirectories: true ) - // The atomic temp+rename happens in the target's directory, so a symlink at - // `url` is written through and preserved instead of replaced. - try data.write(to: target, options: [.atomic]) // Settings may carry secrets (agent profile env overrides can hold API - // keys, docs-ai 053/004); keep every settings file owner-only instead of - // the default 0644. - try FileManager.default.setAttributes( - [.posixPermissions: 0o600], - ofItemAtPath: target.path(percentEncoded: false) - ) + // keys, docs-ai 053/004): the temporary file is born 0600, and the + // same-directory rename(2) both replaces the target atomically — through a + // symlink at `url`, preserving the link — and carries those owner-only + // permissions with it, so the content is never readable by other users, + // not even between write and rename. + let temporary = + target + .deletingLastPathComponent() + .appending(path: ".\(target.lastPathComponent).tmp-\(UUID().uuidString)", directoryHint: .notDirectory) + let temporaryPath = temporary.path(percentEncoded: false) + guard + fileManager.createFile( + atPath: temporaryPath, + contents: data, + attributes: [.posixPermissions: 0o600] + ) + else { + throw SymlinkPreservingFileWriterError.temporaryFileCreationFailed(target) + } + guard rename(temporaryPath, target.path(percentEncoded: false)) == 0 else { + let code = errno + try? fileManager.removeItem(at: temporary) + throw SymlinkPreservingFileWriterError.renameFailed(target, code: code) + } + } + + /// Best-effort owner-only migration for files written before saves enforced + /// 0600 (docs-ai 053/004). Runs on load so a legacy 0644 settings file stops + /// being world-readable the first time it is touched, not on its next save. + static func restrictToOwnerOnly(_ url: URL) { + guard let target = try? resolvedTarget(for: url) else { return } + let path = target.path(percentEncoded: false) + let fileManager = FileManager.default + guard + let permissions = (try? fileManager.attributesOfItem(atPath: path))?[.posixPermissions] as? Int, + permissions != 0o600 + else { return } + try? fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: path) } /// macOS resolves at most MAXSYMLINKS (32) links before ELOOP, so a deeper diff --git a/supacode/Features/Settings/Views/AgentProfileEditorView.swift b/supacode/Features/Settings/Views/AgentProfileEditorView.swift index 07accedc..4a7d0269 100644 --- a/supacode/Features/Settings/Views/AgentProfileEditorView.swift +++ b/supacode/Features/Settings/Views/AgentProfileEditorView.swift @@ -159,7 +159,7 @@ struct AgentProfileEditorView: View { .help("Add an environment variable override for this profile's launches") } label: { Text("Environment Variables") - Text("Applied to the launched process, on top of the shell's environment.") + Text("Applied to the new terminal surface — its shell and everything started in it.") } } diff --git a/supacodeTests/AgentProfileTests.swift b/supacodeTests/AgentProfileTests.swift index f3ffe52d..b60a9fa2 100644 --- a/supacodeTests/AgentProfileTests.swift +++ b/supacodeTests/AgentProfileTests.swift @@ -242,6 +242,7 @@ struct AgentProfileTests { AgentProfileEnvironmentOverride(name: "PROWL_WORKTREE_PATH", value: "/forged"), AgentProfileEnvironmentOverride(name: "CODEX_HOME", value: "/elsewhere"), AgentProfileEnvironmentOverride(name: "CLAUDE_CONFIG_DIR", value: "/elsewhere"), + AgentProfileEnvironmentOverride(name: "HOME", value: "/relocated"), AgentProfileEnvironmentOverride(name: "NUL_VALUE", value: "trunc\0ated"), ] @@ -310,6 +311,14 @@ struct AgentProfileTests { for: AgentProfileEnvironmentOverride(name: "CLAUDE_CONFIG_DIR", value: "x") ) == .reservedName ) + // Relocating `HOME` would move every runtime's default home past the + // dedicated-home safeguards, so it is reserved alongside the account-home + // variables (docs-ai 053/005). + #expect( + AgentProfileEnvironmentPolicy.issue( + for: AgentProfileEnvironmentOverride(name: "HOME", value: "/relocated") + ) == .reservedName + ) #expect( AgentProfileEnvironmentPolicy.issue( for: AgentProfileEnvironmentOverride(name: "OK", value: "bad\0value") diff --git a/supacodeTests/SymlinkPreservingFileWriterTests.swift b/supacodeTests/SymlinkPreservingFileWriterTests.swift index fc233551..aa6c070a 100644 --- a/supacodeTests/SymlinkPreservingFileWriterTests.swift +++ b/supacodeTests/SymlinkPreservingFileWriterTests.swift @@ -194,4 +194,48 @@ struct SymlinkPreservingFileWriterTests { #expect(try Data(contentsOf: url) == payload) #expect(!isSymlink(url)) } + + @Test func writeLeavesNoTemporaryFilesBehind() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let url = dir.appending(path: "settings.json", directoryHint: .notDirectory) + + try SymlinkPreservingFileWriter.write(Data("{}".utf8), to: url) + + let contents = try fileManager.contentsOfDirectory(atPath: dir.path(percentEncoded: false)) + #expect(contents == ["settings.json"]) + } + + @Test func overwritingLegacyPermissionsEndsOwnerOnly() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let url = dir.appending(path: "settings.json", directoryHint: .notDirectory) + try Data("old".utf8).write(to: url) + try fileManager.setAttributes( + [.posixPermissions: 0o644], ofItemAtPath: url.path(percentEncoded: false) + ) + + try SymlinkPreservingFileWriter.write(Data("new".utf8), to: url) + + let attributes = try fileManager.attributesOfItem(atPath: url.path(percentEncoded: false)) + #expect(attributes[.posixPermissions] as? Int == 0o600) + } + + @Test func restrictToOwnerOnlyMigratesLegacyFileThroughSymlink() throws { + let dir = try makeTempDir() + defer { try? fileManager.removeItem(at: dir) } + let target = dir.appending(path: "target.json", directoryHint: .notDirectory) + let link = dir.appending(path: "settings.json", directoryHint: .notDirectory) + try Data("{}".utf8).write(to: target) + try fileManager.setAttributes( + [.posixPermissions: 0o644], ofItemAtPath: target.path(percentEncoded: false) + ) + try fileManager.createSymbolicLink(at: link, withDestinationURL: target) + + SymlinkPreservingFileWriter.restrictToOwnerOnly(link) + + let attributes = try fileManager.attributesOfItem(atPath: target.path(percentEncoded: false)) + #expect(attributes[.posixPermissions] as? Int == 0o600) + #expect(isSymlink(link)) + } }