Harden security and improve shell environment handling

[amirimatin]

Short summary

This PR updates pre-release with the final integrated hardening state for the recent lshell security work.

The branch includes:

1. the bareword path ACL hardening work
2. the SCP parsing and runtime-environment hardening work
3. the deterministic command resolution and command-path drift protection work
4. the fail-closed change for legacy SFTP passthrough
5. the already-merged branch content that this hardening set depends on, including the current completion, prompt, and documentation state

This PR is intentionally aggregated because these changes overlap in the command parser, runtime executor, SSH command handling, hardening templates, and tests. Reviewing them as one final branch is cleaner than asking upstream to merge several partially overlapping topic branches in a specific order.

Repository and component background

lshell is a restricted shell. Its core job is to accept a user command, check it against command policy and path policy, and then execute only what the policy allows.

In the current codebase, the relevant execution path for this PR crosses these main areas:

1. configuration loading and policy finalization in lshell/config/runtime.py
2. command parsing, resolution, and execution helpers in lshell/utils.py
3. runtime authorization and execution flow in lshell/engine/executor.py
4. SSH command dispatch in lshell/shellcmd.py
5. hardening profile generation in lshell/hardeninit.py

This matters because the reviewed issues were not all in one place. Some were in path handling, some in SSH protocol routing, and some in runtime command lookup.

Why this PR is one integrated branch

Earlier work was split across several topic branches and PRs. That was useful during development, but it became less useful once the changes started overlapping in behavior and test coverage.

In this final branch state:

1. the command execution model was hardened
2. several security-sensitive tests changed their expectations
3. hardening profiles and docs were updated to match the new runtime behavior
4. the release and pre-release branches were aligned

If upstream reviewed these as independent PRs, merge order and conflict handling would become part of the review burden. This PR removes that burden and presents the final branch state exactly as it was validated.

Main change area 1: deterministic command resolution and drift detection

Plain-language summary

Before this change, lshell could still depend on the inherited ambient PATH when resolving allowed bare commands such as ls.

After this change, lshell builds its own trusted runtime PATH, resolves bare commands at session start, pins them to a concrete executable, and fails closed if the resolved target changes during the session.

Why this matters

If a restricted shell trusts an inherited PATH, a user or parent process can influence which executable is launched for an allowlisted command name.

That is especially dangerous when policy says "ls is allowed" but the actual executable reached by ls can change due to environment manipulation or an executable swap after session start.

Conditions required for the old risk

The old risk existed when all of the following were true:

1. the policy allowed a bare command name such as ls
2. the runtime inherited an untrusted or attacker-controlled PATH
3. command resolution still depended on that inherited search order

Step-by-step execution path after the fix

1. configuration loading finalizes a trusted runtime path in lshell/config/runtime.py:625-669
2. the same config flow builds a command-resolution cache in lshell/config/runtime.py:87-93 and lshell/config/runtime.py:426-431
3. trusted command resolution is built in lshell/utils.py:385-500
4. each bare command is revalidated against the pinned record in lshell/utils.py:503-591
5. execution denies drift with a dedicated policy reason in lshell/engine/executor.py:120-135 and lshell/engine/executor.py:389-445

What the code now does

The hardening chain is explicit:

• lshell/utils.py:385-407 builds a deterministic runtime PATH
• lshell/utils.py:437-470 resolves a command into path plus metadata
• lshell/utils.py:473-500 caches approved bare-command targets
• lshell/utils.py:503-525 compares the current resolution to the pinned record
• lshell/utils.py:528-591 rewrites the executed command so the pinned executable is used
• lshell/engine/executor.py:396-410 denies execution if the command path changed

Practical impact

This closes two related problems:

1. inherited PATH hijack for bare allowlisted commands
2. command target drift after session start

It does not block explicit path commands such as /usr/bin/foo or ./script.sh. That is intentional. This change is targeted at bare command resolution hardening, not at redefining the semantics of explicit paths.

Main change area 2: SCP parsing hardening

Plain-language summary

The old SCP routing logic could misclassify transfer direction because it relied on simple string matching for " -f " and " -t ".

That left room for clustered short options such as -pf or -pt to bypass upload or download restrictions.

Why this matters

If scp_download=0, the policy is supposed to prevent the server-side download flow. If option clustering hides the -f direction flag from policy detection, the restriction is unreliable.

Step-by-step execution path

1. SSH command handling reaches the SCP branch in lshell/shellcmd.py:406-410
2. the command is classified before trusted execution is allowed
3. the resulting runtime command then executes under the hardened executor and environment model

This PR description does not repeat the full parser internals line by line because the SCP hardening is already integrated into the validated branch state, but this change is part of the same reviewed update set and is covered by the test results listed below.

Practical impact

This closes a policy bypass where clustered short options could start a forbidden SCP direction even though the administrator explicitly disabled it.

Main change area 3: legacy SFTP now fails closed by default

Plain-language summary

The old SFTP passthrough model let lshell launch sftp-server even though lshell cannot inspect each file operation once the SFTP protocol takes over.

This means lshell path ACLs are not a real SFTP filesystem boundary.

The new behavior is explicit and safer:

1. if sftp=1 and sftp_unsafe_legacy=1, legacy passthrough is still allowed
2. if sftp=1 without that override, lshell now refuses legacy passthrough
3. the recommended secure model is sshd_config ForceCommand internal-sftp with ChrootDirectory

Why this matters

This is not a cosmetic change. It changes the default security stance from "implicitly allow a model that cannot enforce its claimed path boundary" to "fail closed unless the administrator explicitly accepts the risk."

Conditions required for the old risk

The old risk existed when:

1. sftp=1 was enabled
2. the administrator expected lshell path ACL to govern actual SFTP file requests
3. the real filesystem boundary was not enforced by SSH-side controls

Step-by-step execution path after the fix

1. the SSH command string is inspected in lshell/shellcmd.py:354-359
2. lshell detects whether the requested protocol command is one of the trusted SFTP executables in lshell/shellcmd.py:368-370
3. if sftp_unsafe_legacy=1 is present, the trusted protocol path is allowed in lshell/shellcmd.py:370-380
4. if the override is absent, the request is denied in lshell/shellcmd.py:381-395
5. hardening profile defaults keep the unsafe legacy path disabled in lshell/hardeninit.py:72-87, lshell/hardeninit.py:206-209, and lshell/hardeninit.py:299-305

What this change does not mean

This change does not make legacy sftp-server passthrough path-safe.

It means the unsafe legacy behavior is no longer the implicit default. If an administrator still wants it, they must explicitly opt back into it with:

sftp_unsafe_legacy : 1

Practical impact

This is a fail-closed design correction, not a protocol proxy.

That distinction matters for review. The security improvement is not "we can now inspect SFTP requests." The real improvement is "we no longer silently allow a mode that suggests stronger containment than the code can actually enforce."

Main change area 4: hardening profiles and docs now match runtime reality

Plain-language summary

A secure runtime model is only useful if the generated profiles and the docs describe the same thing the code does.

This branch updates the hardening templates and docs to match the new fail-closed SFTP model and the new command-resolution model.

Step-by-step effect

1. the sftp-only profile now ships with sftp: 0 and sftp_unsafe_legacy: 0 in lshell/hardeninit.py:72-87
2. field comments explain the risk of the legacy mode in lshell/hardeninit.py:203-210
3. validation rejects unsafe assumptions for the sftp-only profile in lshell/hardeninit.py:299-305
4. supporting docs and sample configuration were updated in the integrated branch state

Practical impact

This reduces operator confusion. An administrator reading the generated hardening output is less likely to assume that legacy SFTP passthrough is the recommended restricted-SFTP deployment model.

Validation and test status

The integrated branch was validated with the full local suite:

.venv/bin/pytest -q

Result:

562 passed, 66 subtests passed in 112.78s

One important detail matters here.

During the full-suite run after integration, several older tests still assumed that user-visible command strings would always appear as bare executable names such as ls, sleep, or tail.

After command pinning, some user-visible output can legitimately contain pinned absolute command paths such as /usr/bin/ls or /usr/bin/sleep.

Because of that, the final integrated branch includes a test-only alignment commit:

• 549e4d5 test: normalize expectations for pinned command paths

This commit does not weaken the hardening. It only updates old expectations so they match the new execution model.

Important review notes

This PR is broader than one isolated security patch

This PR carries the final integrated pre-release state, not a minimal cherry-pick set.

That means reviewers should expect:

1. security hardening changes
2. profile and documentation changes
3. supporting test expectation changes
4. already-integrated branch content that is part of the validated final state

The most important security design decisions to review

If a reviewer wants to focus on the high-signal parts first, these are the best targets:

1. deterministic runtime path construction in lshell/utils.py:385-414
2. command pinning and drift denial in lshell/utils.py:437-591 and lshell/engine/executor.py:389-445
3. legacy SFTP refusal without explicit override in lshell/shellcmd.py:368-395
4. hardened sftp-only profile defaults in lshell/hardeninit.py:72-87 and lshell/hardeninit.py:299-305

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.