Skip to content

Commit

Permalink
[antlir] Forbid directly setting UIDs/GIDs
Browse files Browse the repository at this point in the history
Summary:
> I'm in favor of removing explicit id numbers from the feature api, it's just a footgun that we don't need or want
> -- vmagro @ 03/12/2024 09:20 PT

Test Plan: Green signals mean nothing is filling these fields anymore

Reviewed By: vmagro

Differential Revision: D67791738

fbshipit-source-id: 1b2629fd8a21e15e54329f0f206849109bcf3780
  • Loading branch information
pzmarzly authored and facebook-github-bot committed Jan 6, 2025
1 parent 72dfb36 commit 46f1dbc
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 38 deletions.
4 changes: 0 additions & 4 deletions antlir/antlir2/features/group/group.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ load("//antlir/buck2/bzl:ensure_single_output.bzl", "ensure_single_output")
def group_add(
*,
groupname: str | Select,
gid: int | Select | None = None,
uidmap: str = "default"):
"""
Add a group entry to /etc/group
Expand All @@ -30,7 +29,6 @@ def group_add(
"uidmap": ("antlir//antlir/uidmaps:" + uidmap) if ":" not in uidmap else uidmap,
},
kwargs = {
"gid": gid,
"groupname": groupname,
},
)
Expand All @@ -42,7 +40,6 @@ def _impl(ctx: AnalysisContext) -> list[Provider]:
FeatureAnalysis(
feature_type = "group",
data = struct(
gid = ctx.attrs.gid,
groupname = ctx.attrs.groupname,
uidmap = uidmap,
),
Expand All @@ -55,7 +52,6 @@ def _impl(ctx: AnalysisContext) -> list[Provider]:
group_rule = rule(
impl = _impl,
attrs = {
"gid": attrs.option(attrs.int()),
"groupname": attrs.string(),
"plugin": attrs.exec_dep(providers = [FeaturePluginInfo]),
"uidmap": attrs.dep(),
Expand Down
16 changes: 3 additions & 13 deletions antlir/antlir2/features/group/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use antlir2_users::group::GroupRecord;
use antlir2_users::group::GroupRecordPassword;
use antlir2_users::uidmaps::UidMap;
use antlir2_users::GroupId;
use antlir2_users::Id;
use anyhow::Context;
use serde::Deserialize;
use serde::Serialize;
Expand All @@ -26,7 +25,6 @@ pub type Feature = Group;

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub struct Group {
pub gid: Option<u32>,
pub groupname: GroupName,
pub uidmap: BuckOutSource,
}
Expand All @@ -39,7 +37,7 @@ impl antlir2_depgraph_if::RequiresProvides for Group {
}

fn requires(&self) -> Result<Vec<Requirement>, String> {
let _ = get_gid(&self.gid, &self.uidmap, &self.groupname).map_err(|e| format!("{e:#}"))?;
get_gid(&self.uidmap, &self.groupname).map_err(|e| format!("{e:#}"))?;
Ok(vec![Requirement::ordered(
ItemKey::Path(std::path::Path::new("/etc/group").into()),
Validator::Exists,
Expand All @@ -51,11 +49,10 @@ impl antlir2_compile::CompileFeature for Group {
#[tracing::instrument(skip(ctx), ret, err)]
fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> {
let mut groups_db = ctx.groups_db()?;
let gid = get_gid(&self.gid, &self.uidmap, &self.groupname)?;
let record = GroupRecord {
name: self.groupname.to_owned().into(),
password: GroupRecordPassword::X,
gid,
gid: get_gid(&self.uidmap, &self.groupname)?,
users: Vec::new(),
};
groups_db.push(record)?;
Expand All @@ -64,14 +61,7 @@ impl antlir2_compile::CompileFeature for Group {
}
}

fn get_gid(
supplied_gid: &Option<u32>,
uidmap: &BuckOutSource,
groupname: &GroupName,
) -> anyhow::Result<GroupId> {
if let Some(gid) = supplied_gid {
return Ok(GroupId::from_raw(*gid));
}
fn get_gid(uidmap: &BuckOutSource, groupname: &GroupName) -> anyhow::Result<GroupId> {
let uidmap = UidMap::load(uidmap)?;
uidmap
.get_gid(groupname)
Expand Down
8 changes: 0 additions & 8 deletions antlir/antlir2/features/user/user.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def user_add(
username: str | Select,
primary_group: str | Select,
home_dir: str | Select,
uid: int | Select | None = None,
uidmap: str = "default",
shell: str | Select = SHELL_NOLOGIN,
supplementary_groups: list[str | Select] | Select = [],
Expand Down Expand Up @@ -68,16 +67,13 @@ def user_add(
"primary_group": primary_group,
"shell": shell,
"supplementary_groups": supplementary_groups,
"uid": uid,
"username": username,
},
)

def standard_user(
username: str,
groupname: str,
uid: int | None = None,
gid: int | None = None,
uidmap: str = "default",
home_dir: str | None = None,
shell: str = SHELL_BASH,
Expand All @@ -92,15 +88,13 @@ def standard_user(
return [
group_add(
groupname = groupname,
gid = gid,
uidmap = uidmap,
),
user_add(
username = username,
primary_group = groupname,
home_dir = home_dir,
shell = shell,
uid = uid,
uidmap = uidmap,
supplementary_groups = supplementary_groups,
),
Expand All @@ -125,7 +119,6 @@ def _impl(ctx: AnalysisContext) -> list[Provider]:
primary_group = ctx.attrs.primary_group,
shell = ctx.attrs.shell,
supplementary_groups = ctx.attrs.supplementary_groups,
uid = ctx.attrs.uid,
uidmap = uidmap,
username = ctx.attrs.username,
),
Expand All @@ -144,7 +137,6 @@ user_rule = rule(
"primary_group": attrs.string(),
"shell": attrs.string(),
"supplementary_groups": attrs.list(attrs.string()),
"uid": attrs.option(attrs.int()),
"uidmap": attrs.dep(),
"username": attrs.string(),
},
Expand Down
16 changes: 3 additions & 13 deletions antlir/antlir2/features/user/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use antlir2_features::types::UserName;
use antlir2_users::passwd::UserRecord;
use antlir2_users::passwd::UserRecordPassword;
use antlir2_users::uidmaps::UidMap;
use antlir2_users::Id;
use antlir2_users::UserId;
use anyhow::Context;
use serde::Deserialize;
Expand All @@ -33,7 +32,6 @@ pub type Feature = User;
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub struct User {
pub username: UserName,
pub uid: Option<u32>,
pub uidmap: BuckOutSource,
pub primary_group: GroupName,
pub supplementary_groups: Vec<GroupName>,
Expand All @@ -50,7 +48,7 @@ impl antlir2_depgraph_if::RequiresProvides for User {
}

fn requires(&self) -> Result<Vec<Requirement>, String> {
let _ = get_uid(&self.uid, &self.uidmap, &self.username).map_err(|e| format!("{e:#}"))?;
get_uid(&self.uidmap, &self.username).map_err(|e| format!("{e:#}"))?;
let mut v = vec![
Requirement::unordered(
ItemKey::Path(self.home_dir.to_owned()),
Expand Down Expand Up @@ -80,11 +78,10 @@ impl antlir2_compile::CompileFeature for User {
#[tracing::instrument(name = "user", skip(ctx), ret, err)]
fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> {
let mut user_db = ctx.user_db()?;
let uid = get_uid(&self.uid, &self.uidmap, &self.username)?;
let record = UserRecord {
name: self.username.clone().into(),
password: UserRecordPassword::Shadow,
uid,
uid: get_uid(&self.uidmap, &self.username)?,
gid: ctx.gid(&self.primary_group)?,
comment: self.comment.clone().unwrap_or("".to_owned()).into(),
homedir: self.home_dir.to_owned().into(),
Expand Down Expand Up @@ -117,14 +114,7 @@ impl antlir2_compile::CompileFeature for User {
}
}

fn get_uid(
supplied_uid: &Option<u32>,
uidmap: &BuckOutSource,
username: &UserName,
) -> anyhow::Result<UserId> {
if let Some(uid) = supplied_uid {
return Ok(UserId::from_raw(*uid));
}
fn get_uid(uidmap: &BuckOutSource, username: &UserName) -> anyhow::Result<UserId> {
let uidmap = UidMap::load(uidmap)?;
uidmap
.get_uid(username)
Expand Down

0 comments on commit 46f1dbc

Please sign in to comment.