Skip to content

feat: config trust boundary, honest command feedback, json print mode - #46

Open
mike-diff wants to merge 4 commits into
mainfrom
feat/config-trust-and-feedback
Open

feat: config trust boundary, honest command feedback, json print mode#46
mike-diff wants to merge 4 commits into
mainfrom
feat/config-trust-and-feedback

Conversation

@mike-diff

Copy link
Copy Markdown
Owner

Three related changes, one per commit, each grounded in experiments run in
this repository against the real binary. The through-line: things a checked-out
repo or a raw exit code could make sesh say or send are now made honest.

1. Project config cannot steer the brain (fix(providers))

The hole, measured

A scratch repo whose .sesh/providers.json names a different default, run
against a local listener with a clean HOME:

exit code: 1
request hit listener: True
path: /v1/chat/completions
our prompt reached the attacker endpoint: True

The whole conversation is silently routed to whatever URL the repo names. The
exit 1 is irrelevant: the prompt left on the first POST, and the attacker
needed no key. Two more paths shared the hole: brief_provider resolves
through the project-overlaid config (repl.go briefWriter), so a project
tuning.json routes handoff briefs, transcripts included, the same way; and a
fresh session adopts its brain BY NAME from the last session in the directory,
so a project file redefining a trusted global profile name poisons every future
explicit -provider in that directory.

This contradicted the repo's own rules three times over: tool mods mount
global-only because "a project-local one in a repo you just cloned would be
someone else's code"; the project mcp.json overlay may select but never
define; and "a project providers.json is safe to commit" is true for keys,
not for routing.

The rule

Enforced in the parser, with loud stderr notes for every refusal:

  • the project layer may add profiles (a repo pinning its team's gateway
    stays one flag away: -provider company-gw works exactly as before)
  • the project layer may not set default
  • the project layer may not override a global profile name
  • project tuning.json may not set brief_provider/brief_model;
    every other dial keeps normal layering (thresholds are repo-safe)

/provider add, /provider remove, and the global file are untouched.

Honest cost: a team whose members relied on a project default now passes one
flag. That is the boundary working, not a regression to paper over.

Proof

The exfiltration run, repeated against the fixed binary:

attacker endpoint hit: False
refusal note on stderr: True

2. Honest command feedback (feat(tools) + feat(bash))

Secret masking, ordered before anything is kept

bash env, cat .env, a git remote printed with its embedded token shipped
values to the provider verbatim; the README's standing answer was "bash
remains a hole." Masking now runs inside shape(), FIRST: before the
head/tail split and before the spill write, so the model's shaped copy AND the
spilled file in ~/.sesh/out/ both hold the masked text. Masking after the
spill would persist the secret; masking only the model copy leaves the
read-back path leaky.

Pattern discipline, chosen so ordinary output survives:

  • assignment values masked when the KEY is sensitive: the compound spellings
    (password, passwd, api_key, apikey, private_key, access_key) match as
    substrings; the bare words token and secret must be a whole underscore
    segment, so GITHUB_TOKEN matches while tokenizer_path does not. Bare
    key is never matched, so PATH and MONKEY_HOME survive untouched.
  • quoting survives around the mask: API_KEY="[redacted]", not
    API_KEY=[redacted]
  • well-known token shapes (sk-, ghp_/gho_/ghu_/github_pat_, AKIA
    • 16, xox*) masked wherever they appear, with anchors that do not eat
      prose ("the sk-fork of the repo" survives)
  • one-way. No recovery mechanism, none needed: the model never requires the
    true value, only to know one was there.

Scope is automatic: shaped() wraps exactly bash, the engines, and tool mods,
so masking reaches arbitrary-output tools and nothing else. read/search
results are untouched: file content the user pointed the agent at is a
different, deliberate act. Dial: result_mask_off (inverted default-on, like
result_spill_off), because a false positive that breaks a legitimate
workflow must be turnable off without a recompile.

Benign exit notes

grep -q needle file exits 1 when nothing matched: that is the command
ANSWERING, not failing. Both bash paths returned the bare exit status 1,
the model read failure, and the classic flail followed: pointless re-runs,
tool switching, breakage reports for things that never broke.

Now a small table appends what exit 1 MEANS for that program (grep, diff,
cmp, test, [, pgrep), while the result stays an error result. The note
teaches semantics without lying about status; exit 2 and up stay bare.

3. -p -json (feat(print))

Print mode already had an exit-code contract and a clean stdout; this adds
its machine-readable twin, the "third set of hooks" AGENTS.md anticipated:

{"reply":"...","exit_code":0,"outcome":"done","provider":"local","model":"m",
 "session":"...","iterations":2,"tool_calls":14,"mutations":3,
 "usage":{"input":12000,"output":3400,"cache_read":800}}

One line on stdout in every outcome, failures included: a failed run emits
{"outcome":"error","error":"..."} rather than dying to stderr, so a pipe
stays parseable no matter how the run ended. -json without -p is refused
with usage. blocked shares exit 0 with done in text mode, so the
outcome field is the only way to tell them apart; that is why it exists.
Progress notices stay on stderr (stdout purity is the invariant; stderr is
the human channel).

Tests

Every test was run against a surgically reverted behavior to confirm it
fails. Eleven breakers across the three changes:

Breaker Result
project default accepted again fails
project name-override accepted again fails
project brief_model accepted again fails
masking removed from shape fails
key-rule removed (shape regex still catches ghp_) fails
segment rule dropped fails
quotes swallowed by the mask fails
grep note dropped from table fails
every code annotated (exit 2 too) fails
failure path emits stderr only (json) fails
json scenarios without the flag fail

Two pre-existing tests asserted the old behavior and were updated with
breaker comments rather than deleted: TestTuningBriefDials (its project
layer case asserted the routing hole) and TestProvidersOverlay (its comment
described the project overlay; the primitive is now the global merge).

Three test-written tests caught three real bugs during development, which is
the rule working: quotes were initially swallowed by the mask;
tokenizer_path was initially masked by substring matching; and the first
key-rule test could not fail because a ghp_ value is masked by the token
shapes independently, so it was strengthened with a value no shape catches.

Verification

gofmt clean, go vet ./... clean, go test -race ./... green, full
SESH_E2E=1 suite green, on this branch as assembled. E2E coverage includes
the poisoned-config scenario (the run proceeds on the user's own default,
both refusals loud, and the model that served is the mock's, not the poisoned
name's) and both JSON scenarios (success envelope naming the serving brain;
failure still parseable).

Real-binary smoke: -json without -p exits 2 with usage; a no-provider run
emits a parseable error envelope.

Non-goals

  • No reversible unmasking.
  • No masking in read/search results (deliberate scope, stated boundary).
  • No change to /provider flows or the global config file.
  • No JSON schema machinery beyond the flat envelope.

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