Repository navigation
fix(omp): deny a git push ref in a variable instead of asking - #121
Merged
Merged
Conversation
omp stopped every session on an approval dialog when the agent wrote `git push origin "$B"`. The matcher reads the text before the shell expands it, so it cannot tell a branch name from +main or --force, and omp never applies an allow rule to a line that holds `$`. An allow would let a variable force a push past every literal guard. Deny the four `$` and backtick rules instead of prompting. The agent reads "Blocked by bash pattern", writes the ref literally, and pushes without a question. The deployed omp instructions name `git push -u origin HEAD` for the current branch. Each rule now starts with `*`, so a git push after a shell keyword (`do` in a loop), a wrapper (`env`), or a path (`/usr/bin/git`) no longer runs unchecked. Retire the old prompt copies through OMP_RETIRED_BASH_RULES: the merge keys rules by approval, so it would keep them as user prompts. Refs #120
Code review round 1 and its self-review found that the four first-word deny rules missed a `$` inside a ref, such as `HEAD:$B`. Unquoted, such a value splits into more words and can add `--force`. A prompt put a person in front of the first try; a deny returns the refusal to the agent, whose next try could be that form. Replace the four rules with `*git push *$*` and `*git push *`*`. The wider match costs only an agent retry, never a question. Repaired review findings, round 1: - spec-and-tests MEDIUM (class: untested first-argument guards): add DENIED rows `git push $REMOTE main` and `git push `git remote` main`. - self-review correctness LOW, mid-word refs: the two wider rules, and DENIED rows `git push origin HEAD:$B` and `git push origin "feat/$X"`. - self-review correctness LOW, "line" vs "command": SoT/.omp/AGENTS.md, command-guards whole-line gap, and CHANGELOG say "command". - self-review correctness and text LOW, text false positive: gap bullet with `git commit -F`, CHANGELOG names it, DENIED row for a commit message that names git push before a `$`. - self-review spec LOW, overclaiming sentence: command-guards opening paragraph matches the rules. - self-review text LOW, deny-block header comment names the new denies. - self-review text LOW, "a refusal such as" in command-guards. - self-review text LOW, Output Standard sentence limits: CHANGELOG, AGENTS.md, config.yml comment, command-guards paragraphs split. - self-review text LOW, pre-existing force-push gap: command-guards gap bullet; the force-push rules stay unchanged. Refs #120
Code review round 2 found that no row exercised the widened backtick deny: restoring the word-start backtick rules, or dropping the leading `*`, left the suite green while such a push ran unchecked. Repaired review findings, round 2: - spec-and-tests MEDIUM (class: untested backtick match widening): add DENIED rows ``git push origin HEAD:`git branch --show-current` `` (mid word) and ``env git push origin `git branch --show-current` `` (wrapper prefix). The `$` rule already has its mid-word (`HEAD:$B`, `"feat/$X"`) and prefix (`env`, loop) rows. Refs #120
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.
Yolo run: #120 omp: deny git push with a $ or backtick ref instead of asking the user
Pull request: #121. Checks: all 8 pass (check ×2, lint, golden-dryrun, golden-mutation, portable on ubuntu-24.04, windows-2025, and macos-26). Reviewed head:
3b2df10.What changed
$or a backtick anywhere aftergit pushin the same command, instead of asking a person. The agent readsBlocked by bash pattern: *git push *$*, writes the ref literally, and pushes without a question (SoT/.omp/config.yml).*, so agit pushafter a shell keyword (doin a loop), a wrapper (env), or a path (/usr/bin/git) no longer runs unchecked. A$inside a ref, such asHEAD:$B, is denied too (SoT/.omp/config.yml).git pushprompt rules from the deployed ompconfig.yml(cli/src/engine-native/ompRemovals.ts).git push -u origin HEADfor the current branch and say to rungit pushas its own command (SoT/.omp/AGENTS.md).command-guardstopic, the changelog, version 0.22.5, the SoT payload, and the mutation goldens follow.Decisions and corrections
git pushcommands with a ref in a shell variable, so omp runs them without a prompt. Did: deny them, so the agent rewrites the ref literally and no person is asked. Reason: the matcher reads$Bbefore the shell expands it and cannot tell a branch name from+mainor--force; an allow would let a variable force a push past every literal guard. omp also never applies an allow rule to a command that holds$.$or backtick aftergit push, not only a word that starts with one, because a deny sends the agent to retry and a mid-word form (HEAD:$B, unquoted) can still split into--force. The cost is an agent retry for a later$in the same command or for message text;git commit -Fand runninggit pushalone avoid it.command-guards.Reviews
Plan review passed in round 2, and the post-pass edits passed in delta round 3. Code review passed in round 3 (round 1 panel with a lens self-review, round 2 panel, round 3 delta); each round's findings were fixed.
Open findings: none.
Verification
bun run checkon the reviewed head3b2df10: exit 0 (unit 1456 passed, 11 skipped; golden-dryrun 49 OK; golden-mutation 73 OK).bun run check:generated: exit 0.bun run test:runtime:posix: exit 0.denyfor the screenshot command, the loop delete,HEAD:$B, and theenvwrapper;allowforgit push -u origin HEAD../docks-kit sync omp --dry-run: exit 0; the prune line lists the four old prompt rules.Blocked by bash pattern: *git push *$*; the agent then rangit push -u origin <branch>and the push succeeded with its upstream set, with no prompt. With the old rules, the same call needed approval.Limits and failures
bun run checkafter the change failed ingolden:mutation(4 hash mismatches for the omp files that this change edits); regenerated goldens pass.portable (macos-26)CI job was cancelled after 15 minutes with no step run. The rerun of that job passed in 1m16s on the same head.git -C <dir> push $bruns, and the force-push rules miss a prefix such asdoorenv.Kept resources
Merge now: none. The local branch is deleted withgit branch -d.Leave pull request open: the branch120-omp-deny-git-push-variable-refs(local and remote) and the open pull request.After merge
deploy_local: run./docks-kit sync ompfrommainon this workstation, so the deployed omp config carries the deny rules. It needs its own question.Closes #120