Skip to content

Commit

Permalink
Add subprocess for sandboxed lockfile generation (#1306)
Browse files Browse the repository at this point in the history
Before this patch lockfile generation would always happen in the CLI's
process, which inevitably applied the sandbox to the process itself
making the execution environment after generation severely limited.

To allow for the CLI and extensions to use sandboxed lockfile generation
without having to spawn a separate process, the generation itself is now
always executed in a separate process if sandboxing is requested.

Closes #1296.
  • Loading branch information
cd-work authored Nov 29, 2023
1 parent 7a5c03a commit e1b8811
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 88 deletions.
16 changes: 16 additions & 0 deletions cli/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,22 @@ pub fn add_subcommands(command: Command) -> Command {
.about("Find all lockfile and manifest paths")
.hide(true),
)
.subcommand(
Command::new("generate-lockfile")
.args(&[
Arg::new("lockfile-type")
.value_name("TYPE")
.required(true)
.help("Lockfile type whose generator will be used")
.value_parser(PossibleValuesParser::new(parse::lockfile_types(true))),
Arg::new("manifest")
.value_name("MANIFEST")
.required(true)
.help("Canonicalized manifest path"),
])
.about("Run lockfile generation inside sandbox and write it to STDOUT")
.hide(true),
)
.subcommand(extensions::command());

#[cfg(unix)]
Expand Down
5 changes: 3 additions & 2 deletions cli/src/bin/phylum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use phylum_cli::commands::sandbox;
#[cfg(feature = "selfmanage")]
use phylum_cli::commands::uninstall;
use phylum_cli::commands::{
auth, extensions, find_lockable_files, group, init, jobs, packages, parse, project, status,
CommandResult, ExitCode,
auth, extensions, find_lockable_files, generate_lockfile, group, init, jobs, packages, parse,
project, status, CommandResult, ExitCode,
};
use phylum_cli::config::{self, Config};
use phylum_cli::spinner::Spinner;
Expand Down Expand Up @@ -162,6 +162,7 @@ async fn handle_commands() -> CommandResult {
#[cfg(unix)]
"sandbox" => sandbox::handle_sandbox(sub_matches).await,
"find-lockable-files" => find_lockable_files::handle_command(),
"generate-lockfile" => generate_lockfile::handle_command(sub_matches),
extension_subcmd => {
extensions::handle_run_extension(Box::pin(api), extension_subcmd, sub_matches).await
},
Expand Down
4 changes: 3 additions & 1 deletion cli/src/commands/extensions/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ async fn parse_lockfile(
lockfile: String,
lockfile_type: Option<String>,
generate_lockfiles: Option<bool>,
sandbox_generation: Option<bool>,
) -> Result<PackageLock> {
// Ensure extension has file read-access.
{
Expand All @@ -390,12 +391,13 @@ async fn parse_lockfile(
let project_root = current_project.as_ref().map(|p| p.root());

// Attempt to parse as requested lockfile type.
let sandbox = sandbox_generation.unwrap_or(true);
let generate_lockfiles = generate_lockfiles.unwrap_or(true);
let parsed = parse::parse_lockfile(
lockfile,
project_root,
lockfile_type.as_deref(),
false,
sandbox,
generate_lockfiles,
)?;

Expand Down
107 changes: 107 additions & 0 deletions cli/src/commands/generate_lockfile.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
//! `phylum generate-lockfile` subcommand.
use std::path::{Path, PathBuf};

use anyhow::{Context, Result};
use birdcage::{Birdcage, Exception, Sandbox};
use clap::ArgMatches;
use phylum_lockfile::LockfileFormat;

use crate::commands::extensions::permissions;
use crate::commands::{CommandResult, ExitCode};
use crate::dirs;

/// Handle `phylum generate-lockfile` subcommand.
pub fn handle_command(matches: &ArgMatches) -> CommandResult {
let lockfile_type = matches.get_one::<String>("lockfile-type").unwrap();
let manifest = matches.get_raw("manifest").unwrap().next().unwrap();
let manifest_path = PathBuf::from(manifest);

// Get generator for the lockfile type.
let lockfile_format = lockfile_type.parse::<LockfileFormat>().unwrap();
let generator = lockfile_format.parser().generator().unwrap();

// Setup sandbox for lockfile generation.
let birdcage = lockfile_generation_sandbox(&manifest_path)?;
birdcage.lock()?;

// Generate the lockfile.
let generated_lockfile = generator
.generate_lockfile(&manifest_path)
.context("lockfile generation subcommand failed")?;

// Write lockfile to stdout.
println!("{}", generated_lockfile);

Ok(ExitCode::Ok)
}

/// Create sandbox with exceptions allowing generation of any lockfile.
fn lockfile_generation_sandbox(canonical_manifest_path: &Path) -> Result<Birdcage> {
let mut birdcage = permissions::default_sandbox()?;

// Allow all networking.
birdcage.add_exception(Exception::Networking)?;

// Add exception for the manifest's parent directory.
let project_path = canonical_manifest_path.parent().expect("Invalid manifest path");
permissions::add_exception(&mut birdcage, Exception::WriteAndRead(project_path.into()))?;

// Add exception for all the executables required for generation.
let ecosystem_bins = [
"cargo", "bundle", "mvn", "gradle", "npm", "pnpm", "yarn", "python3", "pipenv", "poetry",
"go", "dotnet",
];
for bin in ecosystem_bins {
let absolute_path = permissions::resolve_bin_path(bin);
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead(absolute_path))?;
}

// Allow any executable in common binary directories.
//
// Reading binaries shouldn't be an attack vector, but significantly simplifies
// complex ecosystems (like Python's symlinks).
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead("/usr/bin".into()))?;
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead("/bin".into()))?;

// Add paths required by specific ecosystems.
let home = dirs::home_dir()?;
// Cargo.
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead(home.join(".rustup")))?;
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead(home.join(".cargo")))?;
permissions::add_exception(&mut birdcage, Exception::Read("/etc/passwd".into()))?;
// Bundle.
permissions::add_exception(&mut birdcage, Exception::Read("/dev/urandom".into()))?;
// Maven.
permissions::add_exception(&mut birdcage, Exception::WriteAndRead(home.join(".m2")))?;
permissions::add_exception(&mut birdcage, Exception::WriteAndRead("/var/folders".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/opt/maven".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/etc/java-openjdk".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/usr/local/Cellar/maven".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/usr/local/Cellar/openjdk".into()))?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/opt/homebrew/Cellar/maven".into()),
)?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/opt/homebrew/Cellar/openjdk".into()),
)?;
// Gradle.
permissions::add_exception(&mut birdcage, Exception::WriteAndRead(home.join(".gradle")))?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/usr/share/java/gradle/lib".into()),
)?;
permissions::add_exception(&mut birdcage, Exception::Read("/usr/local/Cellar/gradle".into()))?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/opt/homebrew/Cellar/gradle".into()),
)?;
// Pnpm.
permissions::add_exception(&mut birdcage, Exception::Read("/tmp".into()))?;
// Yarn.
permissions::add_exception(&mut birdcage, Exception::Read(home.join("./yarn")))?;

Ok(birdcage)
}
1 change: 1 addition & 0 deletions cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::process;
pub mod auth;
pub mod extensions;
pub mod find_lockable_files;
pub mod generate_lockfile;
pub mod group;
pub mod init;
pub mod jobs;
Expand Down
116 changes: 31 additions & 85 deletions cli/src/commands/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@
use std::borrow::Cow;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::result::Result as StdResult;
use std::{env, fs, io};

use anyhow::{anyhow, Context, Result};
use birdcage::{Birdcage, Exception, Sandbox};
use phylum_lockfile::generator::Generator;
use phylum_lockfile::{LockfileFormat, Package, PackageVersion, Parse, ThirdPartyVersion};
use phylum_types::types::package::{PackageDescriptor, PackageDescriptorAndLockfile};

use crate::commands::extensions::permissions;
use crate::commands::{CommandResult, ExitCode};
use crate::{config, dirs, print_user_failure, print_user_warning};
use crate::{config, print_user_failure, print_user_warning};

/// Lockfile parsing error.
#[derive(thiserror::Error, Debug)]
Expand Down Expand Up @@ -158,7 +157,8 @@ pub fn parse_lockfile(
eprintln!("Generating lockfile for manifest {display_path:?} using {format:?}…");

// Generate a new lockfile.
let generated_lockfile = generate_lockfile(generator, &path, sandbox_generation)?;
let generated_lockfile =
generate_lockfile(generator, format.name(), &path, sandbox_generation)?;

// Parse the generated lockfile.
let packages = parse_lockfile_content(&generated_lockfile, parser)?;
Expand All @@ -167,91 +167,37 @@ pub fn parse_lockfile(
}

/// Generate a lockfile from a manifest inside a sandbox.
fn generate_lockfile(generator: &dyn Generator, path: &Path, sandbox: bool) -> Result<String> {
fn generate_lockfile(
generator: &dyn Generator,
lockfile_type: &str,
path: &Path,
sandbox: bool,
) -> Result<String> {
let canonical_path = path.canonicalize()?;

// Enable the sandbox.
if sandbox {
let birdcage = lockfile_generation_sandbox(&canonical_path)?;
birdcage.lock()?;
}

let generated_lockfile = generator
.generate_lockfile(&canonical_path)
.context("Lockfile generation failed! For details, see: \
https://docs.phylum.io/docs/lockfile_generation")?;

Ok(generated_lockfile)
}

/// Create sandbox with exceptions allowing generation of any lockfile.
fn lockfile_generation_sandbox(canonical_manifest_path: &Path) -> Result<Birdcage> {
let mut birdcage = permissions::default_sandbox()?;

// Allow all networking.
birdcage.add_exception(Exception::Networking)?;

// Add exception for the manifest's parent directory.
let project_path = canonical_manifest_path.parent().expect("Invalid manifest path");
permissions::add_exception(&mut birdcage, Exception::WriteAndRead(project_path.into()))?;

// Add exception for all the executables required for generation.
let ecosystem_bins = [
"cargo", "bundle", "mvn", "gradle", "npm", "pnpm", "yarn", "python3", "pipenv", "poetry",
"go", "dotnet",
];
for bin in ecosystem_bins {
let absolute_path = permissions::resolve_bin_path(bin);
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead(absolute_path))?;
// Spawn separate sandboxed process to generate the lockfile.
let current_exe = env::current_exe()?;
let output = Command::new(current_exe)
.arg("generate-lockfile")
.arg(lockfile_type)
.arg(canonical_path)
.output()?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
Err(anyhow!("subprocess failed:\n{stderr}"))
.context("Lockfile generation failed! For details, see: \
https://docs.phylum.io/docs/lockfile_generation")
} else {
Ok(String::from_utf8_lossy(&output.stdout).into())
}
} else {
generator
.generate_lockfile(&canonical_path)
.context("Lockfile generation failed! For details, see: \
https://docs.phylum.io/docs/lockfile_generation")
}

// Allow any executable in common binary directories.
//
// Reading binaries shouldn't be an attack vector, but significantly simplifies
// complex ecosystems (like Python's symlinks).
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead("/usr/bin".into()))?;
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead("/bin".into()))?;

// Add paths required by specific ecosystems.
let home = dirs::home_dir()?;
// Cargo.
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead(home.join(".rustup")))?;
permissions::add_exception(&mut birdcage, Exception::ExecuteAndRead(home.join(".cargo")))?;
permissions::add_exception(&mut birdcage, Exception::Read("/etc/passwd".into()))?;
// Bundle.
permissions::add_exception(&mut birdcage, Exception::Read("/dev/urandom".into()))?;
// Maven.
permissions::add_exception(&mut birdcage, Exception::WriteAndRead(home.join(".m2")))?;
permissions::add_exception(&mut birdcage, Exception::WriteAndRead("/var/folders".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/opt/maven".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/etc/java-openjdk".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/usr/local/Cellar/maven".into()))?;
permissions::add_exception(&mut birdcage, Exception::Read("/usr/local/Cellar/openjdk".into()))?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/opt/homebrew/Cellar/maven".into()),
)?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/opt/homebrew/Cellar/openjdk".into()),
)?;
// Gradle.
permissions::add_exception(&mut birdcage, Exception::WriteAndRead(home.join(".gradle")))?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/usr/share/java/gradle/lib".into()),
)?;
permissions::add_exception(&mut birdcage, Exception::Read("/usr/local/Cellar/gradle".into()))?;
permissions::add_exception(
&mut birdcage,
Exception::Read("/opt/homebrew/Cellar/gradle".into()),
)?;
// Pnpm.
permissions::add_exception(&mut birdcage, Exception::Read("/tmp".into()))?;
// Yarn.
permissions::add_exception(&mut birdcage, Exception::Read(home.join("./yarn")))?;

Ok(birdcage)
}

/// Attempt to parse a lockfile.
Expand Down
2 changes: 2 additions & 0 deletions extensions/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Added

- `generateLockfiles` parameter for `parseLockfile` to inhibit lockfile generation
- `sandboxGeneration` parameter for `parseLockfile` to disable the lockfile
generation sandbox

### Fixed

Expand Down
2 changes: 2 additions & 0 deletions extensions/phylum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,12 +443,14 @@ export class PhylumApi {
lockfile: string,
lockfileType?: string,
generateLockfiles?: boolean,
sandboxGeneration?: boolean,
): Promise<Lockfile> {
return DenoCore.opAsync(
"parse_lockfile",
lockfile,
lockfileType,
generateLockfiles,
sandboxGeneration,
);
}

Expand Down

0 comments on commit e1b8811

Please sign in to comment.