Skip to content

feat: implement runcwd Defaults option#1542

Open
Sim-hu wants to merge 4 commits into
trifectatechfoundation:mainfrom
Sim-hu:feat/implement-runcwd-defaults-option
Open

feat: implement runcwd Defaults option#1542
Sim-hu wants to merge 4 commits into
trifectatechfoundation:mainfrom
Sim-hu:feat/implement-runcwd-defaults-option

Conversation

@Sim-hu
Copy link
Copy Markdown
Contributor

@Sim-hu Sim-hu commented Apr 7, 2026

Summary

Implements the runcwd sudoers Defaults setting (the Defaults equivalent of the CWD command tag), allowing admins to set the working directory policy globally instead of per-rule.

  • Defaults runcwd=* permits --chdir for all commands (same as CWD=*)
  • Defaults runcwd=/path sets a specific working directory
  • Explicit CWD tags on command specs still take precedence
  • Defaults !runcwd resets to default behavior

Closes #1448

Changes

  • Add runcwd to the defaults! macro as an Option<Box<str>> text setting
  • Store parsed ChDir in Judgement so the DirChange reference lifetime is correct
  • Fall back to runcwd when no CWD tag is present on the matched command spec
  • Core logic change is a single line: tag.cwd.as_ref()tag.cwd.as_ref().or(self.runcwd.as_ref())

Test plan

  • Unit test: runcwd direct manipulation on Judgement (runcwd=*, runcwd=/path, CWD override)
  • Integration test: full sudoers parsing with Defaults runcwd=*, runcwd=/path, CWD override, and !runcwd negation
  • Parser validation: Defaults runcwd = *, Defaults runcwd = /usr/local, Defaults !runcwd
  • All existing sudoers tests pass (42/42)
  • clippy clean

Sim-hu added 3 commits April 7, 2026 23:51
Add support for the `runcwd` sudoers Defaults setting, which is the
Defaults equivalent of the `CWD` command tag. This allows administrators
to configure the working directory policy globally rather than per-rule.

When `tag.cwd` is not set on a command spec, the `runcwd` Defaults
value is used as a fallback. Explicit `CWD` tags still take precedence.

Closes trifectatechfoundation#1448
Comment thread src/sudoers/mod.rs Outdated
pub struct Judgement {
flags: Option<Tag>,
settings: Settings,
runcwd: Option<tokens::ChDir>,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since runcwd is already a part of Settings, I would prefer to not have it duplicated here; I believe it can be fed into the tokens::ChDir::construct function further below. Otherwise things can get confusing.

Comment thread src/sudoers/test/mod.rs Outdated
);
}

#[test]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integration tests are good, but they should go in the test-framework folder so they are also verified for compliance against original sudo. In any case, I find this particular test-case to be a bit "dense".

Copy link
Copy Markdown
Member

@squell squell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in defaults/mod.rs and the Judgement::authorization function are fine.

However, I would like to see:

  • runcwd_defaults_integration_test to be replaced with a proper "full up" compliance test in the test-framework/ folder. As it stands this test looks rather complex (even though the addition of runcwd isn't too complex), and secondly an integration test can be expressed much more succinctly and provide more confidence over there.

  • There's no need to add runcwd to the Judgement object.

Also, I don't know if LLM's were used to generate (parts of) this PR, but if so, take note of our stance on genAI.

…uite

Fold the runcwd default into tag.cwd when no explicit CWD is set instead of
storing it on Judgement, and replace the dense in-tree integration test with a
compliance test under test-framework that is verified against original sudo.
@Sim-hu
Copy link
Copy Markdown
Contributor Author

Sim-hu commented May 30, 2026

Addressed both points:

  • Dropped the runcwd field from Judgement. It's now folded into tag.cwd in check() when no explicit CWD is set, so the existing ChDir::construct path handles it and policy.rs is back to plain tag.cwd.as_ref().
  • Removed the in-tree integration test and added a compliance test instead (test-framework/.../sudoers/runcwd.rs, 3 cases). It passes against both original sudo (1.9.16) and sudo-rs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Q] Is sudoers option "runcwd" implemented?

2 participants