Repository navigation
fix: security fixes for 1.1.4 - #49
Merged
Merged
Conversation
OwnershipMiddleware was attached with r.Use on the /sessions subrouter,
where chi has not matched {sessionID} yet. chi.URLParam returned an empty
string, so the check passed every request and a subscription tenant could
read and act on any other tenant's session by ID.
Mount the {sessionID} routes in their own subrouter and attach the
middleware there, so the param is resolved when the check runs. The
lookup is injectable (SessionOwnership) so the wiring is covered by a
router-level test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitLab note hooks do not carry the author's role, so /review, /fix and /fix-cr on a same-project MR were dispatched for anyone able to comment, including users with no access to the code. /fix forwards the note body as the prompt of a code-writing session. Look up the author's effective access level through the GitLab members API (members/all, so group membership counts) and dispatch only for Developer (30) or above, mirroring the GitHub author_association check. Missing IDs, a missing lookup or an API error refuse the command. The lookup refuses redirects so the token stays on the instance host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The server ran git in workspaces the AI CLI had written to, with the server's full environment. Workspace-controlled configuration (fsmonitor, hooks, filter and diff drivers, include files, the shared ~/.gitconfig) could run programs that received CODEFORGE_* secrets and provider tokens, and a changed origin URL or insteadOf rewrite made the askpass helper hand the push token to another host. All server-side git now goes through git.Command, which: - pins settings that run programs or recurse into submodules with -c (fsmonitor, hooksPath, credential.helper, askPass, submodule recursion, ext/file protocol policy, gpg signing) and sets safe.directory there; - ignores the global config (GIT_CONFIG_GLOBAL=/dev/null); - builds the environment from an allowlist (PATH, HOME, TLS, proxy). SanitizeRepoConfig rebuilds .git/config from an allowlist before git runs in a workspace and refuses a .git that is a symlink, a gitfile or has a commondir. Fetch and push reset origin to the URL the session was cloned from. The askpass script answers only prompts for that scheme and host. Commits and pushes also pass --no-verify, diffs --no-ext-diff --no-textconv. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A tenant session could reach the operator's access in several ways: naming a registered key in provider_key, falling back to the GITHUB_TOKEN/GITLAB_TOKEN of the server when it brought no token, having tool config auto-filled from registered keys, getting the operator's registered MCP servers (with their env and headers) written into its workspace, cloning a local path through a file:// repo_url, or reusing another tenant's workspace through workspace_session_id. applyTenant now rejects provider_key, non-HTTP(S) repo URLs and workspace_session_id values that do not name one of the tenant's own sessions. Sessions with a TenantID (Session.UsesOperatorCredentials) skip the token fallback in the executor and PR service, resolve tools without auto-fill (tools.Resolver.ResolveOwnConfig) and get only their own MCP servers. Operator sessions are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The prompt was the last positional argument of `codex exec` with no "--" in front of it, so a prompt starting with "-" was parsed as an option: "-cmodel_provider=..." became a config override. Pass "--" before the prompt and cover the argument list with a test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security fixes for 1.1.4. Each commit is self-contained and comes with a regression test that fails without it.
Changes
Tenant ownership on session routes (
fix(server))The ownership middleware was mounted where chi had not resolved
{sessionID}yet, so the check never ran. The{sessionID}routes now live in their own subrouter with the middleware attached there. Test:TestSessionRoutes_TenantCannotReachForeignSession(router-level, all nine routes).GitLab MR note commands need Developer access (
fix(webhooks))Note hooks don't carry the author's role, so
/review,/fixand/fix-crwere dispatched for anyone who could comment on a same-project MR. The author's effective access level is now looked up through the members API (members/all) and must be ≥ 30, mirroring the GitHubauthor_associationcheck. Any lookup failure refuses the command. Tests:TestGitLabNoteCommandRequiresDeveloperAccess,TestGitLabAccessLevel(local fake API).Server-side git treats the workspace as untrusted (
fix(git))Git run by the server in a session workspace could be steered by configuration the AI CLI wrote there, and it inherited the server environment. All server-side git now goes through
git.Command, which:-c;SanitizeRepoConfigrebuilds.git/configfrom an allowlist before use and refuses redirected.gitdirectories. Fetch and push use the URL the session was cloned from, and the askpass helper answers only for that host. Tests:TestServerGitIgnoresWorkspaceExecutorsTestPushIgnoresWorkspaceOriginTestCommandEnvironmentExcludesServerSecretsTestSanitizeRepoConfigTestAskPassScriptOnlyAnswersRepoHostTenant sessions don't use operator credentials (
fix(subscription))In subscription mode, a tenant session could end up with operator-owned access through:
All of these are now closed for sessions with a tenant. Operator sessions are unchanged. Tests:
TestApplyTenant_RejectsForeignCredentialsTestResolveToken_TenantSessionGetsNoOperatorTokenTestSetupMCP_TenantSessionGetsNoOperatorCredentialsTestResolveAccessTokenTestResolver_ResolveOwnConfigSkipsAutoFillCodex prompt can't inject options (
fix(runner))--is now passed before the prompt. Test:TestCodexArgs_PromptCannotInjectOptions.Behaviour changes for operators
code_review.default_key_nametoken must be able to read project members (read_api/api). If it can't, note commands are refused.~/.gitconfig. Set TLS trust for self-hosted instances withGIT_SSL_CAINFO/SSL_CERT_FILEor/etc/gitconfig. Dev and action images setsafe.directorythere; it is now passed on the command line instead.provider_key→403;repo_urlmust behttp(s).access_token.Release notes draft (v1.1.4)
🤖 Generated with Claude Code