Skip to content

docs(adr): add ADR 0002 — sec-09 config privilege separation#344

Merged
padak merged 1 commit into
mainfrom
docs/adr-0002-sec-09-privsep
May 25, 2026
Merged

docs(adr): add ADR 0002 — sec-09 config privilege separation#344
padak merged 1 commit into
mainfrom
docs/adr-0002-sec-09-privsep

Conversation

@padak
Copy link
Copy Markdown
Member

@padak padak commented May 25, 2026

What

Converts the free-form docs/issue-271-sec-09-config-privilege-separation.md into a formal ADR — docs/adr/0002-sec-09-config-privilege-separation.md — matching the existing docs/adr/0001 template (Status / Date / Context / Decision / Consequences).

Decision recorded

  • The permissions confirmation guard is guard rails (protects against an agent mistake), not a sandbox. In single-user mode the agent and the policy file share one uid, so no in-process guard is enforceable.
  • A real hard lockout is an OS-level privilege-separation deployment pattern (separate service user owning config.json, sudo-brokered wrapper, scrubbed env, pinned config), never an in-process feature.
  • The robust enforcement should land in a future kbagent "locked mode" (KBAGENT_NO_ADMIN + KBAGENT_LOCKED_CONFIG_DIR) in tested Python, not a defeatable bash allowlist.
  • Do not build a stronger in-process isatty-style guard — wrong layer.

Status: Accepted (the sec-09 decision on #271).

Notes

  • Docs-only; no version bump.
  • Also fixes the stale padak/keboola_agent_cli issue URL → keboola/cli after the repo move.
  • The original free-form doc was untracked (never committed); this PR replaces it with the ADR.

Refs #271 (finding sec-09).


Open in Devin Review

Convert the free-form docs/issue-271-sec-09-config-privilege-separation.md
into a formal ADR (0002) matching the docs/adr/ template (Status / Date /
Context / Decision / Consequences).

Decision: the permissions guard is guard rails (protects against an agent
mistake), not a sandbox; a real hard lockout is an OS-level privilege-
separation deployment pattern, with the robust enforcement landing in a
future kbagent 'locked mode' rather than a defeatable bash allowlist. Do
not build a stronger in-process isatty-style guard.

Also fixes the stale padak/keboola_agent_cli issue URL to keboola/cli
after the repo move.
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment on lines +91 to +94
### What ships now (the sec-09 decision on #271)

- Rewrite the misleading docstring at `permissions.py:34`.
- Ship this ADR as the authoritative "Hardened multi-user setup" reference.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚩 ADR claims docstring rewrite 'ships now' but PR does not include it

The ADR's "What ships now" section (line 93) states: "Rewrite the misleading docstring at permissions.py:34." However, this PR only adds the ADR itself — the docstring at commands/permissions.py:34 still reads "This prevents AI agents from programmatically bypassing permission policies." which is exactly the claim the ADR identifies as misleading. The docstring change may be planned for a companion PR, but as written the ADR's "ships now" claim is not fulfilled by this PR alone.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@padak padak merged commit 78ef62d into main May 25, 2026
3 checks passed
@padak padak deleted the docs/adr-0002-sec-09-privsep branch May 25, 2026 13:47
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.

1 participant