Add CNAPP integration commands and helpers

[jpkeepersecurity]

Title

Add CNAPP integration commands and helpers

Summary

Adds a new pam cnapp ... command tree to Commander that drives the krouter CNAPP (Cloud-Native Application Protection Platform) REST endpoints, giving CLI parity with the vault's Cloud Security view. The initial provider is Wiz; the wire/proto layer is provider-agnostic so additional CNAPP vendors can be added without further CLI changes.

Includes the protobuf definitions, a thin helper layer that owns the wire contract, the argparse-driven command surface, and a unit-test suite that exercises the real proto serializers against a mocked router transport.

Changes

• keepercommander/commands/discoveryrotation.py — register PAMCnappCommand under the pam group with alias cn.
• keepercommander/commands/pam/cnapp_commands.py (new, 608 lines) — full pam cnapp command tree:
  • pam cnapp config set | test | test-encrypter | read | delete — manage the provider configuration and health-check the customer-deployed Encrypter.
  • pam cnapp queue list | associate | remediate | set-status | delete — drive the issue queue (list with optional payload decryption, attach a vault record, dispatch remediation to the gateway, update status, delete).
  • Client-side AES-256-GCM decryption of queue payloads using the Encrypter key resolved from the linked vault record. Wire format mirrors decryptCnappQueueItem in vault/cloudSecurityUtils.ts so the CLI and vault see the same plaintext.
  • --format table|json on read/list commands for scripting.
• keepercommander/commands/pam/cnapp_helper.py (new, 241 lines) — single source of truth for the krouter wire contract. One Python function per endpoint under /api/user/cnapp/, plus enum parsers (provider_from_name, action_from_name) and UID conversion utilities. Callers pass parsed Python types; the helper builds the proto, dispatches via _post_request_to_router, and returns the typed response.
• keepercommander/proto/cnapp_pb2.py (new, 181 lines) — generated protobuf bindings for cnapp.proto, covering CnappConfiguration, queue request/response messages, CnappProvider (currently WIZ), and CnappRemediationAction (ROTATE_CREDENTIALS, MANAGE_ACCESS, JIT_ACCESS, REMOVE_STANDING_PRIVILEGE).
• unit-tests/pam/test_cnapp.py (new, 837 lines) — coverage for enum parsing, helper-level request shaping for every endpoint, command-level argument validation and output rendering (table + JSON), payload decryption (happy path + bad-key + missing-key fallback), and status-name → id resolution.

Cross-references

• Server: krouter routes under /api/user/cnapp/... (configuration/{set,test,test-encrypter,read,delete}, queue, queue/{associate,remediate,set-status,delete}).
• Vault: queue payload envelope and Encrypter-key custom-field label kept in sync with vault/cloudSecurityUtils.ts (CNAPP_ENCRYPTION_KEY_FIELD_LABEL = "Encryption Key", AES-256-GCM with nonce(12) || ciphertext || tag(16)).
• Status enum mirrors CnappQueueStatus from CnappModels.kt / keeper.cnapp_queue_status — new statuses must be added in both places.

Notes / limitations

• krouter currently only dispatches ROTATE_CREDENTIALS; the other remediation actions are wired through the proto but return RRC_BAD_REQUEST server-side until those flows are implemented.
• config read never returns clientSecret; passing an empty --client-secret to config set instructs krouter to keep the previously stored value.
• queue list falls back to showing payloads as encrypted (with a warning) when no Encrypter key can be resolved from the linked vault record; --no-decrypt opts out explicitly.

Test plan

• python -m unittest unit-tests.pam.test_cnapp -v passes locally.
• Against a krouter test environment with a Wiz tenant:
  • pam cnapp config set persists; pam cnapp config read returns the same network/provider/endpoint/client-id (and no secret).
  • pam cnapp config test succeeds with valid creds and fails cleanly with the provider's reason on bad creds.
  • pam cnapp config test-encrypter --url https://... returns OK for a healthy Encrypter and surfaces a clear error otherwise.
  • pam cnapp queue list --network-uid ... renders the table with decrypted summaries (severity · title · resource) when the Encrypter record is in the vault; the same call with --no-decrypt shows encrypted and a warning.
  • pam cnapp queue list --format json emits valid JSON with decryptedPayload populated and the raw payload field stripped.
  • pam cnapp queue associate then pam cnapp queue remediate ... rotate_credentials round-trips through the gateway and queue list reflects the new status.
  • pam cnapp queue set-status pending|in_progress|resolved|failed|cancelled and numeric ids both work; unknown names raise a CommandError listing the valid options.
  • pam cnapp config delete and pam cnapp queue delete succeed and subsequent reads return the expected empty/RRC_BAD_STATE responses.

[smunoz-ks]

@jpkeepersecurity You probably need to rebase this PR's branch KC-1290 onto the release branch and drop the preceding/unrelated commits appearing in this PR.

I'm assuming that this PR should only consist of the 3 commits below, not all 13 that are listed in this PR?

1. be35442 Add CNAPP integration commands and helpers
2. 0f5ab77 Refactor CNAPP helper to lazily import router_helper
3. 9ef4534 Refactor test_cnapp.py to resolve circular import issues and enhance test assertions

[smunoz-ks]

AI Code Review (KC-1290): CNAPP integration commands and helpers

Branch: KC-1290 vs release
Scope reviewed: The three CNAPP-only commits on top of release branch:

• be354421 Add CNAPP integration commands and helpers
• 0f5ab775 Refactor CNAPP helper to lazily import router_helper
• 9ef4534a Refactor test_cnapp.py to resolve circular import issues and enhance test assertions

Files reviewed:

• keepercommander/commands/discoveryrotation.py (+3 lines — wiring)
• keepercommander/commands/pam/cnapp_commands.py (new, 608 lines)
• keepercommander/commands/pam/cnapp_helper.py (new, 258 lines)
• keepercommander/proto/cnapp_pb2.py (new, generated)
• unit-tests/pam/test_cnapp.py (new, 850 lines)

---

Summary

Severity	Count
critical	0
high	0
medium	3
low	17

The PR is well-organized — helpers/commands/tests are cleanly separated and the test coverage (~850 lines) is broad including AES round-trips, JSON shape, and decrypt-failure isolation. The dominant concerns are (a) a fail-open path in the crypto envelope check (#1), (b) a workaround for a pre-existing import cycle that now has two sites depending on it (#2, #3), and (c) an enum mirror that will drift (#6). No blocking defects, but #1 should be fixed before merge and #2/#3/#6 should at minimum get follow-up tickets cited in the code.

---

Findings

1. AES-GCM envelope accepts a missing alg field as implicit AES-256-GCM — Severity: medium

cnapp_commands.py:133:

if envelope.get('alg') and envelope['alg'] != 'AES-256-GCM':
    raise ValueError(f"Unsupported CNAPP payload algorithm: {envelope['alg']}")

The guard short-circuits when alg is missing/None/empty. A payload with {"encrypted_payload": "...", "version": "1"} (no alg) is silently treated as AES-256-GCM. Crypto code should fail closed: a missing algorithm declaration must be rejected, not assumed. Tighten to alg = envelope.get('alg'); if alg != 'AES-256-GCM': raise ValueError(...).

2. Lazy import in cnapp_helper._post_request_to_router papers over a pre-existing record↔ksm cycle — Severity: medium

cnapp_helper.py:45-60:

The module comment correctly identifies that the import chain cnapp_helper → router_helper → gateway_helper → utils → ksm → record → ksm re-enters ksm mid-load. Every other PAM helper (record.py:32, pam_saas/__init__.py:3, pam_cloud/pam_privileged_access.py:16, half a dozen discover/*.py files) imports router_helper at module top with no issue, because production startup loads record first. Working around the cycle in this one helper has two costs:

• The fix is local to CNAPP, so the next module to import router_helper cold will hit the same trap and re-discover the workaround.
• It introduces a non-obvious indentation in mocks: patch.object(cnapp_helper, '_post_request_to_router', ...) works because the wrapper exists, but patch.object(router_helper, '_post_request_to_router', ...) would not affect cnapp_helper calls.

The right fix is to break the underlying record↔ksm cycle. If that's out of scope for this PR, file a follow-up and reference it in the comment so the indirection has an exit plan.

3. Test relies on import-order side effect (import keepercommander.commands.record) — Severity: medium

unit-tests/pam/test_cnapp.py:22:

import keepercommander.commands.record  # noqa: F401, E402 - intentional import-order guard

This is the same circular-import workaround surfacing in the test. The import-order guard is brittle: lint/format tools that reorder imports (isort, black with force_sort_within_sections, IDE auto-organize) will silently break the suite. Either:

• Resolve the underlying cycle (see finding #2) so the guard is unnecessary, or
• Move the guard into a conftest.py fixture so it survives auto-formatting and is consistent across test files.

4. _resolve_status lets arbitrary integers through unchecked — Severity: low

cnapp_commands.py:144-155:

if isinstance(value, int):
    return value
s = str(value).strip().lower()
if s.isdigit():
    return int(s)

• _resolve_status(99) returns 99 even though no status with that ID exists.
• _resolve_status('-1') raises (negative string isn't isdigit), while _resolve_status(-1) passes through. Inconsistent.
• For queue list this is forwarded to krouter; for set-status only the 0 case is rejected explicitly.

Validate that the resolved id is in QUEUE_STATUS_BY_ID (or 0 for list) so the CLI rejects garbage locally rather than relying on a server round-trip for the error message.

5. _load_encrypter_key falls through candidates silently — Severity: low

cnapp_commands.py:101-115:

When the record has multiple candidate fields (a labeled secret, a labeled note, and a generic first note), the loop tries each and returns the first one that decodes to 16/32 bytes. If the labeled "Encryption Key" field is present but contains a malformed value, the function silently falls back to whichever generic note field decodes — which may be unrelated content that coincidentally has 32 bytes of base64. The user has no warning that the wrong field was used.

Recommend: if a candidate labeled Encryption Key exists and fails to decode, log a warning and stop — don't substitute another field's content.

6. Mirror table QUEUE_STATUS_BY_NAME will drift silently from the server — Severity: low

cnapp_commands.py:52-59:

# Mirror of CnappQueueStatus enum from CnappModels.kt
# (must be added here AND on the server side in sync.)
QUEUE_STATUS_BY_NAME = {
    'pending': 1, 'in_progress': 2, 'resolved': 3, 'failed': 4, 'cancelled': 5,
}

This is the third place this enum is defined (Kotlin model, Commander, vault TS). The comment acknowledges the drift risk but does nothing about it. The other six enums in this PR (CnappProvider, CnappRemediationAction) ship via cnapp_pb2; pull the status enum into the proto too and delete this table. If the server-side schema isn't yours to change in this PR, at minimum cite the source file in the comment and file a follow-up to lift it into the proto.

7. Dead import in cnapp_commands.py — Severity: low

cnapp_commands.py:41:

from keeper_secrets_manager_core.utils import bytes_to_base64, url_safe_str_to_bytes

url_safe_str_to_bytes is not referenced anywhere in the file. Remove it.

8. Misleading warning text when --no-decrypt is set — Severity: low

cnapp_commands.py:431-433:

if encrypter_key is None and not kwargs.get('no_decrypt'):
    print(... "Pass --config-record <UID> or run after `pam cnapp config read` succeeds." ...)

The guard correctly suppresses the warning when the user explicitly opted out of decryption, but the table still renders <encrypted> cells. A user passing --no-decrypt may be surprised the column says "encrypted" rather than something neutral like <skipped>. Cosmetic, but the column meaning depends on whether decryption was attempted; consider conditionalizing the cell label on no_decrypt.

9. JSON output silently drops decrypt failures — Severity: low

cnapp_commands.py:415-425:

In JSON mode, a queue item that failed decryption shows up without a decryptedPayload key and without any indicator of why. Automation can't distinguish "no payload" from "payload existed but decryption failed". Include a decryptError (or payloadStatus: 'decrypt_failed') field per item so callers can branch on it. (Table mode does print decrypt errors via decrypt_errors — JSON should be at least as informative.)

10. --client-secret "" as a sentinel for "keep existing" is a poor UX — Severity: low

cnapp_commands.py:201-211, cnapp_helper.py:138-144:

The set command requires --client-secret, but users editing only an endpoint must pass --client-secret "" for krouter to splice in the previously stored secret. This is invisible from --help, easy to forget, and indistinguishable from a typo. Cleaner alternatives:

• Make --client-secret optional and infer "keep existing" when omitted.
• Add an explicit --keep-secret flag and reject the combination --keep-secret --client-secret X.

11. _print_configuration omits empty fields — Severity: low

cnapp_commands.py:601-608:

print(f"  Network UID    : {bytes_to_base64(config.networkUid) if config.networkUid else ''}")

Always-empty networkUid should never happen in a saved configuration, but the conditional hides the asymmetry. If the server returns a configuration without one of these fields, the user gets a blank value rather than an explicit (none) marker. Minor — show (none) to make missing values obvious in human output.

12. _decode_aes_key only accepts 16/32-byte keys — Severity: low (informational)

cnapp_commands.py:68-83:

This is correct for AES-128/AES-256, but the helper is consumed only by _decrypt_cnapp_payload, which is hard-coded to AES-256-GCM via the envelope's alg field. A 16-byte key would decode successfully here, then fail with a less-clear error inside AESGCM. Either reject 16-byte keys at decode time (since we know we only support AES-256-GCM), or document the 16-byte branch as forward-compatible.

13. default_verb = 'read' makes pam cnapp config always error — Severity: low

cnapp_commands.py:198:

PAMCnappConfigCommand defaults to read, which requires --network-uid and --provider. A bare pam cnapp config invocation therefore errors with an argparse message about missing required arguments rather than printing a help summary. Either default to a no-arg subcommand that prints help, or override the group to print help text when no subcommand is given.

14. No pagination beyond status filter on queue list — Severity: low

cnapp_commands.py:334-348, cnapp_helper.py:191-198:

The proto has hasMore, but neither helper nor command exposes an offset/limit. If a single status bucket (e.g. pending) holds more items than the server's page size, the user has no way to reach the rest. Help text says "narrow with --status to page", which is misleading — that's filtering, not paging. Either add proper paging or change the help text to say items beyond the page are unreachable until earlier ones are resolved.

15. Field-name asymmetry in the proto: cnappConfigRecordUid vs cnappConfigurationRecordUid — Severity: low (informational)

cnapp_pb2.py (DESCRIPTOR):

CnappConfiguration.cnappConfigRecordUid and CnappRemediateRequest.cnappConfigurationRecordUid refer to the same concept but use different field names. This makes the helper's parameter list confusing (set_cnapp_configuration(..., cnapp_config_record_uid=...) calls into one field, remediate_cnapp_queue_item(..., cnapp_config_record_uid=...) into the differently-named other). Owner of the .proto source should normalize the field names before this contract is widely consumed.

16. _post_request_to_router indirection breaks production stack traces — Severity: low

cnapp_helper.py:53-60:

Because every CNAPP request goes through the thin wrapper, every traceback now includes an extra frame and every log line that references _post_request_to_router will resolve to cnapp_helper rather than router_helper. Minor, but consider importing the symbol at module top once router_helper is reachable (after the cycle in finding #2 is resolved).

17. --provider default of 'wiz' quietly assumes a single CNAPP provider — Severity: low (informational)

cnapp_commands.py:340-341:

queue list --provider defaults to wiz for the config-record lookup. As more providers are added, anyone running queue list on a non-Wiz network without --provider will silently get the wrong (or no) configuration record, producing <encrypted> cells with no obvious cause. Either require --provider once a second provider lands, or store the queue item's provider id and infer from that.

18. reason and pwd_complexity accept arbitrary strings with no validation — Severity: low (informational)

cnapp_commands.py:489-499, cnapp_helper.py:229-230:

--pwd-complexity is documented as "Password complexity JSON" but passed through verbatim as a string. If the user supplies malformed JSON, krouter discovers it; the CLI could validate with json.loads first and surface a friendly error.

19. _format_timestamp swallows oversized/negative epoch values — Severity: low (informational)

cnapp_commands.py:158-165:

datetime.fromtimestamp(..., tz=timezone.utc).isoformat() raises OSError / ValueError on out-of-range inputs (e.g. server-side bug pushing epoch_ms = -1 or astronomical values), which the code catches and converts to str(epoch_ms). That's fine, but the user sees raw millis with no indicator that the value was invalid. Consider rendering <invalid timestamp: 12345> so misbehavior is visible.

20. Test depends on _decrypted_summary's field-priority ordering — Severity: low (informational)

test_cnapp.py:806-833 (test_decrypt_failure_keeps_other_rows_and_reports):

The test's "good" payload lacks a control.name, so the summary falls through to issue.id ("wiz-good"). The comment in the test acknowledges this is intentional. The test will silently start passing for the wrong reason if _decrypted_summary later prefers issue.severity first. Pin the test to a payload that exercises the priority path explicitly (include both control.name and issue.id, assert the one that should win).