Skip to content

Commit e770357

Browse files
committed
Handle cases where terminal config we want to touch is symlinked
1 parent a6fac78 commit e770357

File tree

2 files changed

+45
-10
lines changed

2 files changed

+45
-10
lines changed

src/interceptors/terminal/terminal-scripts.ts

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as util from 'util';
44
import * as os from 'os';
55
import * as path from 'path';
66

7-
import { canAccess, writeFile, renameFile, readFile } from '../../util/fs';
7+
import { canAccess, writeFile, renameFile, readFile, getRealPath } from '../../util/fs';
88
import { reportError } from '../../error-tracking';
99
import { OVERRIDE_BIN_PATH } from './terminal-env-overrides';
1010

@@ -18,14 +18,21 @@ const SHELL = (process.env.SHELL || '').split('/').slice(-1)[0];
1818
const appendOrCreateFile = util.promisify(fs.appendFile);
1919
const appendToFirstExisting = async (paths: string[], forceWrite: boolean, contents: string) => {
2020
for (let path of paths) {
21-
// Small race here, but end result is ok either way
22-
if (await canAccess(path)) {
23-
return appendOrCreateFile(path, contents);
21+
// Follow the path through symlinks (relatively common for terminal config):
22+
const realPath = await getRealPath(path);
23+
if (!realPath) continue; // File (or linked file) does not exist
24+
25+
if (await canAccess(realPath)) {
26+
// Found our first valid readable file - append our extra config
27+
return appendOrCreateFile(realPath, contents);
2428
}
29+
30+
// ^ Small races here, if the file content/perms change between check and write, but
31+
// unlikely to be a major concern - we'll just fail to write, it'll work next time.
2532
}
2633

2734
if (forceWrite) {
28-
// If force write is set, write the last file anyway
35+
// If force write is set, write the last file anyway, even though it didn't exist before:
2936
return appendOrCreateFile(paths.slice(-1)[0], contents);
3037
}
3138
};
@@ -146,8 +153,13 @@ export const editShellStartupScripts = async () => {
146153
// git-bash ignoring the inherited $PATH.
147154

148155
// .profile is used by Dash, Bash sometimes, and by Sh:
149-
appendOrCreateFile(path.join(os.homedir(), '.profile'), SH_SHELL_PATH_CONFIG)
150-
.catch(reportError);
156+
appendToFirstExisting(
157+
[
158+
path.join(os.homedir(), '.profile')
159+
],
160+
true, // We always write .profile
161+
SH_SHELL_PATH_CONFIG
162+
).catch(reportError);
151163

152164
// Bash login shells use some other files by preference, if they exist.
153165
// Note that on OSX, all shells are login - elsewhere they only are at actual login time.
@@ -192,8 +204,11 @@ export const editShellStartupScripts = async () => {
192204
const removeConfigSectionsFromFile = async (path: string) => {
193205
let fileLines: string[];
194206

207+
const targetPath = await getRealPath(path); // Follow symlinks, if present
208+
if (!targetPath) return; // File doesn't exist, no need to clean it up
209+
195210
try {
196-
fileLines = (await readFile(path, 'utf8')).split('\n');
211+
fileLines = (await readFile(targetPath, 'utf8')).split('\n');
197212
} catch (e) {
198213
// Silently skip any files we can't read
199214
return;
@@ -211,9 +226,9 @@ const removeConfigSectionsFromFile = async (path: string) => {
211226

212227
// Write & rename to ensure this is atomic, and avoid races here
213228
// as much as we reasonably can.
214-
const tempFile = path + Date.now() + '.temp';
229+
const tempFile = targetPath + Date.now() + '.temp';
215230
await writeFile(tempFile, fileLines.join('\n'));
216-
return renameFile(tempFile, path);
231+
return renameFile(tempFile, targetPath);
217232
};
218233

219234
// Cleanup: strip our extra config line from all config files

src/util/fs.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { promisify } from 'util';
22
import * as fs from 'fs';
3+
import * as path from 'path';
34
import * as tmp from 'tmp';
45
import * as rimraf from 'rimraf';
56
import { lookpath } from 'lookpath';
@@ -9,6 +10,7 @@ import { isErrorLike } from './error';
910
export const statFile = promisify(fs.stat);
1011
export const readFile = promisify(fs.readFile);
1112
export const readDir = promisify(fs.readdir);
13+
export const readLink = promisify(fs.readlink);
1214
export const deleteFile = promisify(fs.unlink);
1315
export const checkAccess = promisify(fs.access);
1416
export const chmod = promisify(fs.chmod);
@@ -19,6 +21,24 @@ export const copyFile = promisify(fs.copyFile);
1921

2022
export const canAccess = (path: string) => checkAccess(path).then(() => true).catch(() => false);
2123

24+
// Takes a path, follows any links present (if possible) until we reach a non-link file. This
25+
// does *not* check that the final path is accessible - it just removes any links en route.
26+
// This will return undefined if a target path does not resolve at all.
27+
export const getRealPath = async (targetPath: string): Promise<string | undefined> => {
28+
while (true) {
29+
try {
30+
const linkTarget = await readLink(targetPath);
31+
// Links are often relative, so we need to resolve them against the link parent directory:
32+
targetPath = path.resolve(path.dirname(targetPath), linkTarget);
33+
} catch (e: any) {
34+
// Target file does not exist:
35+
if (e.code === 'ENOENT') return undefined;
36+
// Not a link, or some other error:
37+
else return targetPath;
38+
}
39+
}
40+
};
41+
2242
export const deleteFolder = promisify(rimraf);
2343

2444
export const ensureDirectoryExists = (path: string) =>

0 commit comments

Comments
 (0)