Skip to content

Commit

Permalink
[native_assets_builder] Don't pass in the whole environment
Browse files Browse the repository at this point in the history
  • Loading branch information
dcharkes committed Dec 3, 2024
1 parent 3113f5e commit 22d51b8
Showing 1 changed file with 37 additions and 2 deletions.
39 changes: 37 additions & 2 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ class NativeAssetsBuildRunner {
null,
hookKernelFile,
packageLayout!,
_filteredEnvironment(_environmentVariablesFilter),
),
);
if (buildOutput == null) return null;
Expand All @@ -450,7 +451,7 @@ class NativeAssetsBuildRunner {
Uri? resources,
PackageLayout packageLayout,
) async {
final environment = Platform.environment;
final environment = _filteredEnvironment(_environmentVariablesFilter);
final outDir = config.outputDirectory;
return await runUnderDirectoriesLock(
[
Expand Down Expand Up @@ -526,6 +527,7 @@ ${e.message}
resources,
hookKernelFile,
packageLayout,
environment,
);
if (result == null) {
if (await dependenciesHashFile.exists()) {
Expand All @@ -550,6 +552,22 @@ ${e.message}
);
}

/// Limit the environment that hook invocations get to see.
///
/// This allowlist lists environment variables needed to run mainstream
/// compilers.
static const _environmentVariablesFilter = {
'ANDROID_HOME', // Needed for the NDK.
'HOME', // Needed to find tools in default install locations.
'PATH', // Needed to invoke native tools.
'PROGRAMDATA', // Needed for vswhere.exe.
'SYSTEMROOT', // Needed for process invocations on Windows.
'TEMP', // Needed for temp dirs in Dart process.
'TMP', // Needed for temp dirs in Dart process.
'TMPDIR', // Needed for temp dirs in Dart process.
'USER_PROFILE', // Needed to find tools in default install locations.
};

Future<HookOutput?> _runHookForPackage(
Hook hook,
HookConfig config,
Expand All @@ -559,6 +577,7 @@ ${e.message}
Uri? resources,
File hookKernelFile,
PackageLayout packageLayout,
Map<String, String> environment,
) async {
final configFile = config.outputDirectory.resolve('../config.json');
final configFileContents =
Expand All @@ -583,6 +602,8 @@ ${e.message}
executable: dartExecutable,
arguments: arguments,
logger: logger,
includeParentEnvironment: false,
environment: environment,
);

var deleteOutputIfExists = false;
Expand Down Expand Up @@ -639,6 +660,12 @@ ${e.message}
}
}

Map<String, String> _filteredEnvironment(Set<String> allowList) => {
for (final entry in Platform.environment.entries)
if (allowList.contains(entry.key.toUpperCase()))
entry.key: entry.value,
};

/// Compiles the hook to kernel and caches the kernel.
///
/// If any of the Dart source files, or the package config changed after
Expand Down Expand Up @@ -666,7 +693,11 @@ ${e.message}
Uri packageConfigUri,
Uri workingDirectory,
) async {
final environment = Platform.environment;
final environment = _filteredEnvironment({
'HOME', // Needed for Dart.
'PUB_CACHE', // Needed for Dart.
'SYSTEMROOT', // Needed for process invocation on Windows.
});
final kernelFile = File.fromUri(
outputDirectory.resolve('../hook.dill'),
);
Expand Down Expand Up @@ -703,6 +734,7 @@ ${e.message}
workingDirectory,
kernelFile,
depFile,
environment,
);
if (!success) {
await dependenciesHashFile.delete();
Expand Down Expand Up @@ -745,6 +777,7 @@ ${e.message}
Uri workingDirectory,
File kernelFile,
File depFile,
Map<String, String> environment,
) async {
final compileArguments = [
'compile',
Expand All @@ -759,6 +792,8 @@ ${e.message}
executable: dartExecutable,
arguments: compileArguments,
logger: logger,
includeParentEnvironment: false,
environment: environment,
);
var success = true;
if (compileResult.exitCode != 0) {
Expand Down

0 comments on commit 22d51b8

Please sign in to comment.