Authz engine

[collin-kierce]

This Authz Engine is meant to improve the process of command search within the destructive_command_guardrail directory. This improves search time by breaking commands into categories based on cheap keywords instead of searching every expensive regex patter if just one keyword is found. Why should "rm -rf /" be looked for if the cheap keyword was git? Since commands are now grouped and loaded in from JSON I moved the sibling force_push_guardrail commands into this same program.

I've also added more obfuscation detection so simple obfuscation methods will be detected. For example r\m -rf / would not be caught previously, now it is! Another big change was to the is_real_command function inside detector.py. This function was causing false negatives with commands like sudo rm -rf /. To fix this I removed the function. This now makes the program susceptible to false positives when an exact malicious command is passed in as a string argument. For example echo "rm -rf /" will be a false positive, but an accepted one.

[GregKinne]

Code Puppy Review — refreshed at 543a481

Recommendation: changes requested. The direction is promising: consolidating duplicate guards is DRY, and policy data in JSON could become a useful downstream extension seam. However, the current implementation introduces a proven false-positive regression, nondeterministic policy selection, and a fail-open configuration path. It also does not yet provide the enforcement semantics implied by the name “Authz engine.”

What I verified

• Targeted detector suite: 106 passed.
• Both JSON policy files parse successfully; changed Python compiles.
• The PR currently has no CI checks attached.
• I found no company names, internal endpoints, enterprise products, or other organization-specific criteria in the public diff. Good—keep private policy files downstream.
• git diff --check reports seven trailing-whitespace defects.

---

Must fix

1. Invalid policy data fails open and can disable the entire guard

detector.py:31-60 loads every JSON file as one operation and raises for malformed JSON, missing keys, or invalid regex. Loading occurs lazily from detect_destructive_command() at detector.py:92-106.

The callback framework catches callback exceptions and converts the result to None. For this shell hook, None means no block. Therefore one malformed downstream/enterprise JSON file can make all destructive commands proceed without this guard.

For a security control, configuration failure cannot silently become authorization success.

Please:

• define and validate a versioned schema;
• test malformed JSON, invalid regex, missing/wrong-typed fields, empty files, and multiple files;
• make policy-load failure explicit and fail closed for policies intended to enforce a deny;
• validate the assembled policy during build/startup as well as in unit tests.

2. Consolidation regresses force-push false-positive behavior

The deleted force-push detector required the Git invocation to appear at a command boundary. The unified detector normalizes each segment and searches for a matching phrase anywhere within it (detector.py:111-127).

I compared origin/main with this PR. Base allows quoted command text passed to output commands or a Python snippet; this PR reports --force and blocks it. The old test_echo_push assertion was deleted and not ported.

This matters in Code Puppy because agents commonly print, document, generate, and test command strings. A safety guard must distinguish command invocation from inert data as reliably as practical.

Please restore command-position awareness and port the old false-positive corpus, including quoted output, unrelated commands, dry-run, pull, tags, -u, grep, empty input, and compound commands.

3. A set makes policy priority nondeterministic

At detector.py:113-125, candidate patterns are placed in a set. This discards JSON definition order even though overlapping patterns require “most specific first.”

I ran the same three inputs under 20 PYTHONHASHSEED values and received eight distinct result combinations. In particular, the lease-specific and includes-specific variants can be reported as generic --force; the root-glob pattern can be reported as the non-glob root pattern.

The latest commit removed exact-result assertions at tests/plugins/test_destructive_command_detector.py:43-49 rather than fixing this behavior. Weakening an assertion is not a valid resolution.

Use an ordered candidate collection (for example, an insertion-ordered deduplication strategy), preserve documented file/group/pattern precedence, and restore exact-match assertions.

4. This is currently a configurable warning/approval guard, not an authorization engine

Every JSON match flows through the existing destructive-command callback:

• register_callbacks.py:54-56 allows users to disable the guard;
• register_callbacks.py:62-64 prompts interactive users;
• register_callbacks.py:101-103 allows the command after approval;
• JSON entries have only regex, name, and description—no enforcement effect.

That is reasonable for generic OSS safety prompts, but it cannot enforce a mandatory enterprise rule such as “an agent may never change endpoint protection.” Please choose and document one honest scope:

1. OSS policy-data refactor only: rename/describe this as a JSON-driven destructive-command guard and explicitly state that mandatory enterprise deny semantics are out of scope; or
2. Authorization foundation: define typed effects such as approval vs deny, precedence, bypass rules, and fail-closed behavior, with callback-level tests.

Do not call this a general authz engine unless it makes a subject/action/resource/effect decision or clearly defines its narrower policy contract.

5. Consolidation is not proven behaviorally equivalent

Before this PR there were 33 destructive-detector test functions plus 30 force-push-detector test functions. The consolidated file has 43 test functions (106 parametrized cases), but much of the force-push allow/false-positive corpus was removed. The new tests mostly prove positive detection, not equivalence.

There are also zero direct tests for:

• load_guardrails_data();
• loading an additional generic JSON file;
• deterministic cross-file precedence;
• callback behavior after consolidation;
• invalid-policy behavior;
• policy files being present in the built wheel.

Please retain/parameterize the old cases against the consolidated detector and add loader, callback, and package-artifact tests. A generic company_policy.json fixture is enough—do not put organization-specific criteria in this repository.

6. The downstream extension contract is implicit and underspecified

detector.py:32-33 scans package-local patterns/*.json. That happens to suggest “add another file to the enterprise build,” but the PR does not define:

• schema/version compatibility;
• where downstream policy comes from;
• whether private files are build-time overlays or runtime configuration;
• file/group/pattern precedence;
• duplicate-name behavior;
• failure behavior;
• whether JSON files are guaranteed to be included in wheel/sdist artifacts.

Document this contract and test a built artifact. Keep company-specific policy and product names in the private enterprise distribution; keep only vendor-neutral schema/examples/tests here.

---

Should fix

7. The detector cleanup requested previously is still outstanding

• GLOBAL_PATTERNS: list[SearchGroup] = None is still an invalid annotation.
• Mutable lazy global state remains at detector.py:92-106; prefer a cached function with an explicit test reset seam.
• SearchGroup is still a hand-written mutable class; use a frozen dataclass (and correct tuple[str, ...] typing).
• found_groups is still a misleading name—it contains patterns, not groups.
• All regexes now use re.IGNORECASE; document and test this behavior change.
• git diff --check reports trailing whitespace, and several lines are needlessly long.

8. The PR narrative does not establish purpose, scope, or performance

The title “Authz engine” is too vague, and the PR body literally contains [truncated]. It does not explain the downstream extension goal, security model, compatibility expectations, or risks of consolidating the force-push guard.

The body claims faster search but provides no benchmark. Normalization and command splitting also add work, so include a small reproducible old-vs-new benchmark or make the performance claim qualitative.

This PR combines JSON extraction, search routing, obfuscation normalization, compound-command parsing, and force-guard consolidation. For easier review, either split these concerns or organize commits/tests so each transformation proves parity before the next behavior change.

---

Resolved / good work

• The existing destructive-command callback does provide the runtime hook for the consolidated force-push patterns, so my earlier concern about there being no replacement callback is resolved. One callback instead of two is the right DRY direction.
• The git restore copy/paste test was corrected.
• The missing test class declarations were restored.
• Obfuscation and compound-command coverage increased.
• No enterprise information is present in the public changes.

Suggested minimal path to approval

1. Preserve ordered matching and restore exact-result assertions.
2. Restore command-boundary behavior and the deleted force-push allow corpus.
3. Add schema/loader tests plus explicit fail-closed handling.
4. Add a vendor-neutral extra-policy fixture and wheel-content test.
5. Clarify whether this is an OSS guardrail refactor or a true enforcement foundation; update the title/body accordingly.
6. Add CI and clean the diff.

The core idea is useful. It just needs the security contract and regression proof to be as solid as the architecture pitch.