Skip to content

Commit

Permalink
Ensure tmpdir usage internally is always the realpath
Browse files Browse the repository at this point in the history
As it turns out os.tmpdir can point to a symlink, which can introduce path ambiguity. Some of our algorithms are sensitive to path equality, and introducing a path with indirection via a symlink can confuse the system. For example shared-internal’s explicit-relative algorithm fails to create the expected relative paths for externals as it attempts to compare /private/var/folders/… with /var/folders/…

The algorithms in question are currently not coupled to the state of the filesystem, and most likely should remain decoupled, this then implies the inputs should be carefully considered and we should not introduce spurious ambiguity where the state enters the system. Given that, ensuring a stable + realpath’d tmpdir is appropriate.
  • Loading branch information
stefanpenner committed Jun 23, 2021
1 parent f0fa9ff commit f2d2049
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 67 deletions.
1 change: 1 addition & 0 deletions packages/compat/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"@babel/traverse": "^7.14.5",
"@babel/types": "^7.14.5",
"@embroider/macros": "0.42.2",
"@embroider/shared-internals": "0.42.2",
"@types/babel__code-frame": "^7.0.2",
"@types/yargs": "^17.0.0",
"assert-never": "^1.1.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/compat/src/compat-addons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { join, relative, dirname, isAbsolute } from 'path';
import { emptyDirSync, ensureSymlinkSync, ensureDirSync, realpathSync, copySync, writeJSONSync } from 'fs-extra';
import { Stage, Package, PackageCache, WaitForTrees, mangledEngineRoot } from '@embroider/core';
import V1InstanceCache from './v1-instance-cache';
import { tmpdir } from 'os';
import { tmpdir } from '@embroider/shared-internals';
import { MovedPackageCache } from './moved-package-cache';
import { Memoize } from 'typescript-memoize';
import buildCompatAddon from './build-compat-addon';
Expand Down Expand Up @@ -248,6 +248,6 @@ export default class CompatAddons implements Stage {
private stableWorkspaceDir(app: V1App) {
let hash = createHash('md5');
hash.update(app.root);
return join(tmpdir(), 'embroider', hash.digest('hex').slice(0, 6));
return join(tmpdir, 'embroider', hash.digest('hex').slice(0, 6));
}
}
4 changes: 2 additions & 2 deletions packages/compat/src/compat-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { sync as resolveSync } from 'resolve';
import { MacrosConfig } from '@embroider/macros/src/node';
import bind from 'bind-decorator';
import { pathExistsSync } from 'fs-extra';
import { tmpdir } from 'os';
import { tmpdir } from '@embroider/shared-internals';
import { Options as AdjustImportsOptions } from '@embroider/core/src/babel-plugin-adjust-imports';
import { getEmberExports } from '@embroider/core/src/load-ember-template-compiler';
import semver from 'semver';
Expand Down Expand Up @@ -374,7 +374,7 @@ class CompatAppAdapter implements AppAdapter<TreeNames> {
// it's important that this is a persistent location, because we fill it
// up as a side-effect of babel transpilation, and babel is subject to
// persistent caching.
externalsDir: join(tmpdir(), 'embroider', 'externals'),
externalsDir: join(tmpdir, 'embroider', 'externals'),
emberNeedsModulesPolyfill,
};
}
Expand Down
4 changes: 2 additions & 2 deletions packages/compat/tests/resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { removeSync, mkdtempSync, writeFileSync, ensureDirSync, writeJSONSync, r
import { join, dirname } from 'path';
import Options, { optionsWithDefaults } from '../src/options';
import sortBy from 'lodash/sortBy';
import { tmpdir } from 'os';
import { tmpdir } from '@embroider/shared-internals';
import { NodeTemplateCompiler, throwOnWarnings } from '@embroider/core';
import { emberTemplateCompilerPath } from '@embroider/test-support';
import { Options as AdjustImportsOptions } from '@embroider/core/src/babel-plugin-adjust-imports';
Expand All @@ -21,7 +21,7 @@ describe('compat-resolver', function () {
) {
let EmberENV = {};
let plugins = { ast: [] };
appDir = realpathSync(mkdtempSync(join(tmpdir(), 'embroider-compat-tests-')));
appDir = realpathSync(mkdtempSync(join(tmpdir, 'embroider-compat-tests-')));
writeJSONSync(join(appDir, 'package.json'), { name: 'the-app' });
let resolver = new Resolver({
root: appDir,
Expand Down
5 changes: 2 additions & 3 deletions packages/core/src/packager.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { AppMeta } from '@embroider/shared-internals';
import { AppMeta, tmpdir } from '@embroider/shared-internals';
import { readFileSync } from 'fs-extra';
import { join } from 'path';
import { tmpdir } from 'os';

// This is a collection of flags that convey what kind of build you want. They
// are intended to be generic across Packagers, and it's up to Packager authors
Expand Down Expand Up @@ -106,5 +105,5 @@ export function getAppMeta(pathToVanillaApp: string): AppMeta {
* This ensures they have exactly the same lifetime as some of embroider's own caches.
*/
export function getPackagerCacheDir(name: string): string {
return join(tmpdir(), 'embroider', name);
return join(tmpdir, 'embroider', name);
}
6 changes: 3 additions & 3 deletions packages/core/tests/packager.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AppMeta, getAppMeta, getPackagerCacheDir } from '../src';
import { writeJSONSync } from 'fs-extra';
import { join } from 'path';
import { tmpdir } from 'os';
import { writeJSONSync, realpathSync } from 'fs-extra';
import { join } from 'path';
import * as tmp from 'tmp';

tmp.setGracefulCleanup();
Expand Down Expand Up @@ -38,6 +38,6 @@ describe('getAppMeta', () => {
describe('getPackagerCacheDir', () => {
test('getting the path to a cache directory', () => {
const cacheDir = getPackagerCacheDir('foo');
expect(cacheDir).toBe(join(tmpdir(), 'embroider', 'foo'));
expect(cacheDir).toBe(join(realpathSync(tmpdir()), 'embroider', 'foo'));
});
});
1 change: 1 addition & 0 deletions packages/shared-internals/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ export { default as Package, V2AddonPackage as AddonPackage, V2AppPackage as App
export { default as PackageCache } from './package-cache';
export { default as babelFilter } from './babel-filter';
export { default as packageName } from './package-name';
export { default as tmpdir } from './tmpdir';
export * from './ember-cli-models';
export * from './ember-standard-modules';
13 changes: 13 additions & 0 deletions packages/shared-internals/src/tmpdir.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { realpathSync } from 'fs-extra';
import { tmpdir } from 'os';

// tmpdir() can point to a symlink such as `/var/folders/9n/...` which points
// to `/private/var/folders/9n/...`. Although these are logically the same
// folder, some algorithms operating on the path will not be aware of this and
// treat them differently. So rather then mixing, let's create a shared tmpdir
// value that has already had its real path derived.
//
// Additionally, it is slightly odd to repeatedly ask for `tmpdir()` when the
// in-process expectation is that it remains stable. So storing it as a value
// here should be safe.
export default realpathSync(tmpdir());
1 change: 1 addition & 0 deletions packages/webpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"@embroider/babel-loader-7": "0.42.2",
"@embroider/babel-loader-8": "0.42.2",
"@embroider/hbs-loader": "0.42.2",
"@embroider/shared-internals": "0.42.2",
"@types/loader-utils": "^2.0.2",
"@types/source-map": "^0.5.7",
"@types/supports-color": "^8.1.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/webpack/src/ember-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
getPackagerCacheDir,
getOrCreate,
} from '@embroider/core';
import { tmpdir } from '@embroider/shared-internals';
import webpack, { Configuration } from 'webpack';
import { readFileSync, outputFileSync, copySync, realpathSync, Stats, statSync, readJsonSync } from 'fs-extra';
import { join, dirname, relative, sep } from 'path';
Expand All @@ -30,7 +31,6 @@ import flatMap from 'lodash/flatMap';
import MiniCssExtractPlugin from 'mini-css-extract-plugin';
import makeDebug from 'debug';
import { format } from 'util';
import { tmpdir } from 'os';
import { warmup as threadLoaderWarmup } from 'thread-loader';
import { Options, BabelLoaderOptions } from './options';
import crypto from 'crypto';
Expand Down Expand Up @@ -517,7 +517,7 @@ const Webpack: PackagerConstructor<Options> = class Webpack implements Packager
// the tmpdir on OSX is horribly long and makes error messages hard to
// read. This is doing the same as String.prototype.replaceAll, which node
// doesn't have yet.
error.message = error.message.split(realpathSync(tmpdir())).join('$TMPDIR');
error.message = error.message.split(tmpdir).join('$TMPDIR');
}
return error;
}
Expand Down
4 changes: 2 additions & 2 deletions test-packages/support/suite-setup-util.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { tmpdir } from 'os';
import { tmpdir } from '@embroider/shared-internals';
import { basename, join, relative, resolve } from 'path';
import { readdirSync, statSync, unlinkSync, writeFileSync } from 'fs-extra';
import execa from 'execa';
Expand All @@ -8,7 +8,7 @@ import execa from 'execa';
// plugins is not parallel-safe. So we give each suite a separate TMPDIR to run
// within.
export function separateTemp(name = `separate${Math.floor(Math.random() * 100000)}`): string {
return join(tmpdir(), name);
return join(tmpdir, name);
}

export function testemConfig() {
Expand Down
56 changes: 5 additions & 51 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5283,15 +5283,7 @@ browserify-zlib@^0.2.0:
dependencies:
pako "~1.0.5"

browserslist@^3.2.6:
version "3.2.8"
resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-3.2.8.tgz#b0005361d6471f0f5952797a76fc985f1f978fc6"
integrity sha512-WHVocJYavUwVgVViC0ORikPHQquXwVh939TaelZ4WDqpWgTX/FsGhl/+P4qBUAGcRvtOgDgC+xftNWWp2RUTAQ==
dependencies:
caniuse-lite "^1.0.30000844"
electron-to-chromium "^1.3.47"

browserslist@^4.0.0, browserslist@^4.14.5, browserslist@^4.16.6:
browserslist@^3.2.6, browserslist@^4.0.0, browserslist@^4.14.0, browserslist@^4.14.5, browserslist@^4.16.6:
version "4.16.6"
resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.16.6.tgz#d7901277a5a88e554ed305b183ec9b0c08f66fa2"
integrity sha512-Wspk/PqO+4W9qp5iUTJsa1B/QrYn1keNCcEP5OvP7WBwT4KaDly0uONYmC6Xa3Z5IqnUgS0KcgLYu1l74x0ZXQ==
Expand Down Expand Up @@ -5541,11 +5533,6 @@ caniuse-lite@^1.0.0, caniuse-lite@^1.0.30001219:
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001236.tgz#0a80de4cdf62e1770bb46a30d884fc8d633e3958"
integrity sha512-o0PRQSrSCGJKCPZcgMzl5fUaj5xHe8qA2m4QRvnyY4e1lITqoNkr7q/Oh1NcpGSy0Th97UZ35yoKcINPoq7YOQ==

caniuse-lite@^1.0.30000844:
version "1.0.30001239"
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001239.tgz#66e8669985bb2cb84ccb10f68c25ce6dd3e4d2b8"
integrity sha512-cyBkXJDMeI4wthy8xJ2FvDU6+0dtcZSJW3voUF8+e9f1bBeuvyZfc3PNbkOETyhbR+dGCPzn9E7MA3iwzusOhQ==

capture-exit@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/capture-exit/-/capture-exit-2.0.0.tgz#fb953bfaebeb781f62898239dabb426d08a509a4"
Expand Down Expand Up @@ -6805,11 +6792,6 @@ ee-first@1.1.1:
resolved "https://registry.yarnpkg.com/ee-first/-/ee-first-1.1.1.tgz#590c61156b0ae2f4f0255732a158b266bc56b21d"
integrity sha1-WQxhFWsK4vTwJVcyoViyZrxWsh0=

electron-to-chromium@^1.3.47:
version "1.3.754"
resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.754.tgz#afbe69177ad7aae968c3bbeba129dc70dcc37cf4"
integrity sha512-Q50dJbfYYRtwK3G9mFP/EsJVzlgcYwKxFjbXmvVa1lDAbdviPcT9QOpFoufDApub4j0hBfDRL6v3lWNLEdEDXQ==

electron-to-chromium@^1.3.723:
version "1.3.752"
resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.752.tgz#0728587f1b9b970ec9ffad932496429aef750d09"
Expand Down Expand Up @@ -10132,21 +10114,7 @@ fastboot-transform@^0.1.0, fastboot-transform@^0.1.3:
broccoli-stew "^1.5.0"
convert-source-map "^1.5.1"

fastboot@^2.0.0, fastboot@^2.0.1:
version "2.0.3"
resolved "https://registry.yarnpkg.com/fastboot/-/fastboot-2.0.3.tgz#0b712e6c590f1b463dc5b12138893bcbbafa2459"
integrity sha512-NNH/o+XhITAQUnW2CC9IDXlcnI74W2BONjtRSRmc01N3uJl/7pcvX9iWTUWu2PYQbQZUBu8HzVFt7GmQ9qw9JQ==
dependencies:
chalk "^2.0.1"
cookie "^0.4.0"
debug "^4.1.0"
najax "^1.0.3"
resolve "^1.8.1"
rsvp "^4.8.0"
simple-dom "^1.4.0"
source-map-support "^0.5.0"

fastboot@^3.1.0:
fastboot@^2.0.0, fastboot@^2.0.1, fastboot@^3.1.0:
version "3.1.2"
resolved "https://registry.yarnpkg.com/fastboot/-/fastboot-3.1.2.tgz#c10a97be3a61fbbf9e4bd8abc43373e8739d1787"
integrity sha512-yvhJfIRd4wWWACk+qjJxQI+WBIQ+pyQyp0/fxrQyA/cYJgZAXOHb+22zXJbJXaPku3fHS+gBl7crwovIkl8bhQ==
Expand Down Expand Up @@ -12671,11 +12639,6 @@ jest@^24.9.0:
import-local "^2.0.0"
jest-cli "^24.9.0"

jquery-deferred@^0.3.0:
version "0.3.1"
resolved "https://registry.yarnpkg.com/jquery-deferred/-/jquery-deferred-0.3.1.tgz#596eca1caaff54f61b110962b23cafea74c35355"
integrity sha1-WW7KHKr/VPYbEQlisjyv6nTDU1U=

jquery@^3.3.1, jquery@^3.4.1, jquery@^3.5.0, jquery@^3.5.1:
version "3.6.0"
resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.6.0.tgz#c72a09f15c1bdce142f49dbf1170bdf8adac2470"
Expand Down Expand Up @@ -14040,15 +14003,6 @@ mz@^2.4.0:
object-assign "^4.0.1"
thenify-all "^1.0.0"

najax@^1.0.3:
version "1.0.7"
resolved "https://registry.yarnpkg.com/najax/-/najax-1.0.7.tgz#706dce52d4b738dce01aee97f392ccdb79d51eef"
integrity sha512-JqBMguf2plv1IDqhOE6eebnTivjS/ej0C/Sw831jVc+dRQIMK37oyktdQCGAQtwpl5DikOWI2xGfIlBPSSLgXg==
dependencies:
jquery-deferred "^0.3.0"
lodash "^4.17.21"
qs "^6.2.0"

nan@^2.12.1:
version "2.14.2"
resolved "https://registry.yarnpkg.com/nan/-/nan-2.14.2.tgz#f5376400695168f4cc694ac9393d0c9585eeea19"
Expand Down Expand Up @@ -15284,7 +15238,7 @@ qs@6.7.0:
resolved "https://registry.yarnpkg.com/qs/-/qs-6.7.0.tgz#41dc1a015e3d581f1621776be31afb2876a9b1bc"
integrity sha512-VCdBRNFTX1fyE7Nb6FYoURo/SPe62QCaAyzJvUjwRaIsc+NePBEniHlvxFmmX56+HZphIGtV0XeCirBtpDrTyQ==

qs@^6.2.0, qs@^6.4.0, qs@^6.9.4:
qs@^6.4.0, qs@^6.9.4:
version "6.10.1"
resolved "https://registry.yarnpkg.com/qs/-/qs-6.10.1.tgz#4931482fa8d647a5aab799c5271d2133b981fb6a"
integrity sha512-M528Hph6wsSVOBiYUnGf+K/7w0hNshs/duGsNXPUCLH5XAqjEtiPGwNONLV0tBH8NoGb0mvD5JubnUTrujKDTg==
Expand Down Expand Up @@ -16076,7 +16030,7 @@ rsvp@^3.0.14, rsvp@^3.0.17, rsvp@^3.0.18, rsvp@^3.0.21, rsvp@^3.0.6, rsvp@^3.1.0
resolved "https://registry.yarnpkg.com/rsvp/-/rsvp-3.6.2.tgz#2e96491599a96cde1b515d5674a8f7a91452926a"
integrity sha512-OfWGQTb9vnwRjwtA2QwpG2ICclHC3pgXZO5xt8H2EfgDquO0qVdSb5T88L4qJVAEugbS56pAuV4XZM58UX8ulw==

rsvp@^4.0.1, rsvp@^4.6.1, rsvp@^4.7.0, rsvp@^4.8.0, rsvp@^4.8.1, rsvp@^4.8.2, rsvp@^4.8.3, rsvp@^4.8.4, rsvp@^4.8.5:
rsvp@^4.0.1, rsvp@^4.6.1, rsvp@^4.7.0, rsvp@^4.8.1, rsvp@^4.8.2, rsvp@^4.8.3, rsvp@^4.8.4, rsvp@^4.8.5:
version "4.8.5"
resolved "https://registry.yarnpkg.com/rsvp/-/rsvp-4.8.5.tgz#c8f155311d167f68f21e168df71ec5b083113734"
integrity sha512-nfMOlASu9OnRJo1mbEk2cz0D56a1MBNrJ7orjRZQG10XDyuvwksKbuXNp6qa+kbn839HwjwhBzhFmdsaEAfauA==
Expand Down Expand Up @@ -16640,7 +16594,7 @@ source-map-support@^0.4.15:
dependencies:
source-map "^0.5.6"

source-map-support@^0.5.0, source-map-support@^0.5.16, source-map-support@^0.5.17, source-map-support@^0.5.6, source-map-support@~0.5.10, source-map-support@~0.5.12, source-map-support@~0.5.19:
source-map-support@^0.5.16, source-map-support@^0.5.17, source-map-support@^0.5.6, source-map-support@~0.5.10, source-map-support@~0.5.12, source-map-support@~0.5.19:
version "0.5.19"
resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.19.tgz#a98b62f86dcaf4f67399648c085291ab9e8fed61"
integrity sha512-Wonm7zOCIJzBGQdB+thsPar0kYuCIzYvxZwlBa87yi/Mdjv7Tip2cyVbLj5o0cFPN4EVkuTwb3GDDyUx2DGnGw==
Expand Down

0 comments on commit f2d2049

Please sign in to comment.