Skip to content

fix(permissions): keep colons in argument values from truncating patterns - #3767

Merged
Sayt-0 merged 5 commits into
docker:mainfrom
parveshsaini:parveshsaini/fix/permission-arg-colon-truncation
Jul 22, 2026
Merged

Sayt-0 merged 5 commits into
docker:mainfrom
parveshsaini:parveshsaini/fix/permission-arg-colon-truncation

Conversation

@parveshsaini

Copy link
Copy Markdown
Contributor

Closes #3766

What

parsePattern split a permission pattern on every :, so any argument value
containing a colon was truncated. A deny rule written against a URL —
fetch:url=https://evil.com/* — parsed as url = "https", which never matches a
real URL, so the rule silently never fired (fail-open). Windows drive paths
(shell:cwd=C:\work\* → C) were corrupted the same way.

This parses the tool name up to the first :key= argument condition, then takes
each value up to the next :key= boundary rather than the next colon, so
colons inside a value are preserved. Tool names with colons
(mcp:github:create_issue) are unaffected — they were already handled by
splitting at the first :key=, and that behaviour is unchanged.

Why

This is the sibling of #3605. That fix made matchGlob's wildcards cross
path separators, but with a colon in the value the pattern never reaches
matchGlob intact — so the same fail-open direction was still reachable for URLs
and Windows paths, which are exactly the kind of value an operator writes a
deny rule against. In non-interactive / auto-approval runs, or alongside an
overlapping allow, a corrupted deny is a real bypass. allow rules with
colon values were broken too, but those fail closed.

Tests

Added to pkg/permissions/permissions_test.go:

  • TestParsePattern rows for URL, Windows-drive, and mixed value-colon +
    second-condition patterns.
  • TestDenyRuleWithColonInValue — end-to-end via CheckWithArgs: a deny rule
    with a URL value now returns Deny, plus a shell-command-with-URL case and a
    colon-free control.

Each new assertion fails against main and passes with this change; the existing
TestParsePattern rows (including mcp:github:create_issue) are untouched.

Verification (in golang:1.26.5 + golangci-lint:v2.12.2, matching CI)

$ go test ./pkg/permissions/...
ok  	github.com/docker/docker-agent/pkg/permissions

$ golangci-lint run ./pkg/permissions/...
0 issues.

$ gofmt -l pkg/permissions/    # (no output)

Confirming the tests are meaningful — reverting only permissions.go to main
while keeping the new tests:

--- FAIL: TestParsePattern/fetch:url=https://example.com/*
        expected: map[string]string{"url":"https://example.com/*"}
        actual  : map[string]string{"url":"https"}
--- FAIL: TestParsePattern/shell:cwd=C:\work\*
        expected: map[string]string{"cwd":"C:\\work\\*"}
        actual  : map[string]string{"cwd":"C"}
--- FAIL: TestDenyRuleWithColonInValue
        expected: 2 (Deny)   actual: 0 (Ask)
        Messages: deny rule with a URL value must block the matching URL

…erns

Signed-off-by: parveshsaini <parveshsaini619@gmail.com>
@parveshsaini
parveshsaini requested a review from a team as a code owner July 21, 2026 05:20
@aheritier aheritier added area/security Authentication, authorization, secrets, vulnerabilities kind/fix PR fixes a bug (maps to fix:). Use on PRs only. status/ready-for-review Ready for implementation, design review, or final review labels Jul 21, 2026
Comment thread pkg/permissions/permissions.go
Signed-off-by: parveshsaini <parveshsaini619@gmail.com>
Sayt-0
Sayt-0 previously approved these changes Jul 21, 2026

@Sayt-0 Sayt-0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good to me !

@Sayt-0
Sayt-0 enabled auto-merge July 21, 2026 09:22
dgageot added 3 commits July 21, 2026 19:12
cutAtArgCondition/isArgKey/isArgKeyByte detected a ':' immediately
followed by a run of word characters and '=', which is exactly the
regexp `:(\w+)=`. Splitting on its matches yields the same tool name
and argument conditions in a third of the code.
Split already yields the tool name and one value per condition, and the
keys come from the same non-overlapping matches, so the index-based
FindAllStringSubmatchIndex bookkeeping (submatch offsets, next-match
lookahead) is unnecessary.
@Sayt-0
Sayt-0 merged commit e276bca into docker:main Jul 22, 2026
8 checks passed
pull Bot pushed a commit to TheTechOddBug/cagent that referenced this pull request Jul 23, 2026
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
docker-agent 1.115.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>This release fixes permission pattern parsing for colon-containing values, adds provider-level compaction model defaults, resolves agent-switch slash commands over HTTP, and includes several bug fixes and refactors.

## What's New

- Adds `X-Cagent-Compacting: 1` HTTP header on session-compaction LLM calls to allow gateway-side policies to distinguish compaction calls from regular chat completions
- Adds support for provider-level `compaction_model` default, so agents sharing a provider no longer need to repeat the setting individually
- Adds an optional `description` field to model config entries for human-readable annotations
- Resolves agent-switch slash commands in `RunSession` for HTTP/REST clients, enabling mid-session agent switches without requiring the two-segment route

## Bug Fixes

- Fixes permission pattern parsing where colons in argument values (e.g. URLs or Windows drive paths) caused patterns to be silently truncated, resulting in `deny` rules never firing
- Fixes A2A tool silently succeeding when a sub-agent stream returns empty; also resolves cross-platform test instability on Windows
- Fixes `compaction_model` references not being resolved during `first_available` reachability checks and environment preflight
- Fixes model description not being preserved across `first_available` resolution and shorthand marshalling
- Fixes agent-switch pre-switch command resolution and rolls back partial switches on failure
- Fixes TUI history-search input not being restyled when a live theme change occurs, leaving stale styling after switching themes
- Fixes `GatewayHTTPOptions` panicking when model options are nil
- Reverts a `WorkingDir` containment check that broke callers running docker-agent as a long-lived daemon pointing sessions at arbitrary paths

## Technical Changes

- Fixes log-injection (CodeQL `go/log-injection`) in `pkg/telemetry/client.go` by replacing string concatenation with structured key-value logging
- Sanitizes string arguments to close remaining log-injection taint paths and closes unbarriered passthrough in `sanitizeLogArgs`
- Refactors permission pattern parsing to use `strings.Cut` and then a regexp-based approach replacing the hand-rolled colon scanner
- Moves Desktop proxy HTTP transport to a new leaf package `pkg/desktop/transport`
- Moves Vertex AI support to `anthropic/vertex` subpackage to reduce transitive dependencies for library consumers
---

## What's Changed
* docs: update CHANGELOG.md for v1.114.0 by @docker-read-write[bot] in docker/docker-agent#3775
* feat: add X-Cagent-Compacting header for session-compaction LLM calls by @dgageot in docker/docker-agent#3777
* refactor: trim transitive dependencies for library consumers by @dgageot in docker/docker-agent#3778
* chore: bump direct Go dependencies by @dgageot in docker/docker-agent#3776
* docs: update docs for config v14 and Mermaid state diagram directions by @aheritier in docker/docker-agent#3779
* fix(codeql): go/log-injection in pkg/telemetry/client.go:24 by @ronan-thibaut-glitch in docker/docker-agent#3755
* chore: bump github.com/anthropics/anthropic-sdk-go from v1.58.0 to v1.58.1 by @dgageot in docker/docker-agent#3781
* fix(permissions): keep colons in argument values from truncating patterns by @parveshsaini in docker/docker-agent#3767
* feat(config): add description attribute to models by @dgageot in docker/docker-agent#3783
* feat: support provider-level compaction_model default by @dgageot in docker/docker-agent#3785
* fix: A2A response error handling and cross-platform test stability by @Piyush0049 in docker/docker-agent#3745
* fix(tui): restyle history-search input on live theme change by @aheritier in docker/docker-agent#3786
* feat(server): resolve agent-switch slash commands in RunSession by @trungutt in docker/docker-agent#3782
* Revert "fix(codeql): go/path-injection in pkg/server/session_manager.go:542" (#3758) by @trungutt in docker/docker-agent#3788


**Full Changelog**: docker/docker-agent@v1.114.0...v1.115.0
</pre>
  <p>View the full release notes at <a href="https://github.com/docker/docker-agent/releases/tag/v1.115.0">https://github.com/docker/docker-agent/releases/tag/v1.115.0</a>.</p>
</details>
<hr>

See merge request: Harmonybrew/homebrew-core!14843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/security Authentication, authorization, secrets, vulnerabilities kind/fix PR fixes a bug (maps to fix:). Use on PRs only. status/ready-for-review Ready for implementation, design review, or final review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Permission rules fail open when an argument value contains : (parsePattern truncates the value)

4 participants