🛡️ Sentinel: Fix path traversal and symlink hijacking in nexuscli

[google-labs-jules]

This PR hardens nexuscli/core/api.py against path traversal attacks and symlink permission hijacking:

1. Validates and sanitizes the account parameter in _cache_path to prevent path traversal vectors via account names.
2. Enforces canonical directory alignment checks with os.path.commonpath.
3. Checks is_symlink() before running chmod on configuration directories, files, and cache paths to protect against symlink dereference hijacking on multi-user systems.
4. Adds unit tests in tests/test_nexuscli_privileges.py verifying privilege enforcement, symlink safety, and path traversal prevention.

---

PR created automatically by Jules for task 1144520540131189380 started by @timerloggedout-spec

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
termux-monorepo	Ready	Preview, v0	Aug 17, 2026 4:25pm

[blocksorg]

Mention Blocks like a regular teammate with your question or request:

@blocks review this pull request
@blocks make the following changes ...
@blocks create an issue from what was mentioned in the following comment ...
@blocks explain the following code ...
@blocks are there any security or performance concerns?

Run @blocks /help for more information.

Workspace settings | Disable this message

[github-actions]

head_sha: 459d065
ready: false
autofix_requested: false
timed_out: true

Peer review gate (ready for second-pass agents)

External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot.
Autofix (if any) was requested in a separate comment on this SHA.

Peer activity (truncated):

no peer activity yet

Downstream: gemini-after-peers. Jules: agent-review-auto-jules.

[github-actions]

🔀 OpenRouter review (cohere/north-mini-code:free)

We need to perform a second pass review as a free-tier SECOND-PASS reviewer. We need to be concise, one response, focusing on gaps peers missed (security 0o600/0o700, Class 3/4 artifacts, AGENTS.md); residual risks before merge.

The diff shows changes in nexuscli/core/api.py and a new test file tests/test_nexuscli_privileges.py.

We need to review the changes: the modifications address security issues: ensuring CONFIG_DIR permissions are set correctly, adding symlink checks, sanitizing account names and session_id for path traversal, setting directory/file permissions to 0o700 and 0o600, and not following symlinks for chmod.

We also need to consider any potential gaps: maybe checking that symlink target permissions are not changed? The test test_nexuscli_privileges_symlink_safety ensures that symlink doesn't modify target file's mode.

Potential gaps: Did they ensure that any other directories like ~/.nexuscli may be symlink? The changes seem to guard against symlinks before chmod.

But there might be missing security aspects: The changes enforce permissions only for non-symlink directories/files, but they do not enforce that the entire path hierarchy is secure? For instance, what about the base store directory? The parent directories may be world writable. Also, they do not enforce that the CONFIG_DIR permissions are 0o700 for the root directory; they also add symlink check.

Also there is a new test file; we need to ensure test coverage for security (0o600/0o700). There are three tests: test_nexuscli_privileges_enforcement, test_nexuscli_privileges_symlink_safety, test_nexuscli_privileges_path_traversal_prevention.

Check if any Class 3/4 artifacts? That might be classification concerns? Possibly we need to look at repository's classification levels. Might be related to security classification of code. The PR might have certain artifact classes. But I think this is about "Class 3/4 artifacts" meaning maybe some files are classifiable? Not sure.

Also AGENTS.md might refer to documentation for agents? The peer gate mentions "AGENTS.md". Might be that AGENTS.md exists but hasn't been updated. The diff doesn't show changes to AGENTS.md. Possibly there is an AGENTS.md file in repo. Should we check? Not possible without browsing entire repo. The instruction says "Focus: gaps peers missed (security 0o600/0o700, Class 3/4 artifacts, AGENTS.md)". So we should ensure that security 0o600/0o700 is fully addressed. The changes look correct. Check for potential bug: When CONFIG_DIR is not a symlink but permissions cannot be set due to permission errors, they silently pass. That's okay but may be too silent. Could log a warning but not necessary.

Potential missing: The code does not set permissions on directory ".nexuscli" for sessions? Actually they set parent directory of store_dir and store_dir, but also earlier they set CONFIG_DIR's chmod only if not symlink. They do not set permission on ~/.nexuscli/session_store itself? They set store_dir and parent directories. Actually they also set permission on CONFIG_DIR earlier? Let's examine.

Original code:

CONFIG_DIR.mkdir(parents=True, exist_ok=True)
try:
    CONFIG_DIR.chmod(0o700)
except Exception:
    pass

Now updated:

CONFIG_DIR.mkdir(parents=True, exist_ok=True)
if not CONFIG_DIR.is_symlink():
    try:
        CONFIG_DIR.chmod(0o700)
    except Exception:
        pass

So they added symlink guard for CONFIG_DIR.

Then for cache storage, they compute store_dir = join(base_store, safe_account). base_store = realpath(...). safe_account sanitizes account name. Then they call os.makedirs(store_dir, exist_ok=True) and then chmod parent_store and store_dir if not symlink.

But they do not set the mode of base_store (the parent of store_dir) to 0o700 if not already secure? However earlier CONFIG_DIR's parent? Actually base_store = realpath(os.path.expanduser("~/.nexuscli/session_store")). This path is inside CONFIG_DIR? Wait CONFIG_DIR is set earlier as Path.home() / ".nexuscli"? Let's check in api.py at top? Not provided but likely earlier. So base_store is inside CONFIG_DIR. CONFIG_DIR ensures its mode is 0o700, but the session_store subdirectory's parent directories may have insecure permissions (if they are not symlinks, we should also ensure they are 0o700). However, if CONFIG_DIR's mode is 0o700, then its subdirectories default to inheriting? Not guaranteed on some OS, but it's still recommended to set directory permissions. However, when we create store_dir and parent directories, they are created with default umask, but we can set chmod on each directory as we do.

Thus we have security coverage.

Potential gaps: The code sanitizes account and session_id but does it also sanitize the resulting file name? They only sanitize account characters, but session_id is passed as file name? Wait session_id is used as file name? Actually _cache_path returns path = something? In function:

def _cache_path(session_id: str, account: str = "primary") -> str:
    if ".." in str(account) or str(account).startswith("/") or "\\" in str(account):
        raise ValueError("Invalid account name")

    base_store = os.path.realpath(os.path.expanduser("~/.nexuscli/session_store"))
    safe_account = "".join(c if c.isalnum() or c in "-_." else "_" for c in str(account))
    store_dir = os.path.join(base_store, safe_account)

    store_real = os.path.realpath(store_dir)
    if os.path.commonpath([base_store, store_real]) != base_store:
        raise ValueError("Invalid account path")

    os.makedirs(store_dir, exist_ok=True)
    ...

    if ".." in str(session_id) or str(session_id).startswith("/") or "\\" in str(session_id):
        raise ValueError("Invalid file path")
    # The path is store_dir + "/" + session_id + ".json"? Actually earlier code: maybe path = os.path.join(store_dir, f"{session_id}.json")

We need to see original code: In the diff after changes, they did not show the part of _cache_path where they compute path for session file. Let's search in diff after change: There is no mention of session file path. Might have been omitted. The original file before changes probably had:

store_dir = os.path.join(os.path.expanduser("~/.nexuscli/session_store"), account)
...
def _cache_path(session_id: str, account: str = "primary") -> str:
    store_dir = os.path.join(os.path.expanduser("~/.nexuscli/session_store"), account)
    ...
    path = os.path.join(store_dir, f"{session_id}.json")

But not included in diff. So maybe the existing code omitted generation of file path. However, current diff only shows modifications for config and path traversal checks. Let's open the file content (though diff truncated). However, we only have part of the diff; truncated parts may have other changes. The diff truncated but we see modifications for _cache_path, including path sanitization and checking symlink etc. But we need to ensure session file name sanitization and any final path building. The provided snippet ends at raising errors for account name and safe_account. Not the session file name.

But the diff shows modifications for _cache_save etc. We need to examine complete _cache_path, but the diff didn't include rest. It might be truncated after raising errors.

Given we are limited to second-pass review, we need to summarize any residual risks. Probably the changes address security, but there might be a risk: The safe_account sanitization could still allow path traversal if account contains ".." after sanitization? But they also check for ".." earlier and raise. But safe_account might be something like "abc..def"? Actually they check before sanitization. So okay.

Potential risk: The safe_account sanitization doesn't limit length, could allow symlink attacks if account string leads to a symlink inside store_dir? They check store_dir is not a symlink before chmod, but they don't check if store_dir is a symlink (i.e., a symlink folder). They check parent_store and store_dir for symlink before chmod but they still use os.makedirs if missing. However, if store_dir is a symlink, it would be followed by os.path.realpath for validation, but os.makedirs may create the symlink itself? That might cause risk.

Specifically, in _cache_path they do:

base_store = os.path.realpath(os.path.expanduser("~/.nexuscli/session_store"))
safe_account = sanitized...

---
_Peer router: Omni ↔ OpenRouter by desired model; Gemini residual. role=review_

[timerloggedout-spec]

✅ Merged to master by OPERATOR (Grok).

SHA: a5590340be3a0d76f2c716a66cd4c1efe5eec6cb

Sentinel nexuscli path/symlink harden is live. GitLab status remains non-blocking.

BIUDL 🚀