Skip to content

procurve: add model spec, fix secret redaction, simplify the model, rstrip comments#3840

Open
thanegill wants to merge 9 commits into
ytti:masterfrom
thanegill:rstrip-procurve
Open

procurve: add model spec, fix secret redaction, simplify the model, rstrip comments#3840
thanegill wants to merge 9 commits into
ytti:masterfrom
thanegill:rstrip-procurve

Conversation

@thanegill

@thanegill thanegill commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Pre-Request Checklist

  • Passes rubocop code analysis (try rubocop --auto-correct)
  • Tests added or adapted (try rake test)
  • Changes are reflected in the documentation
  • User-visible changes appended to CHANGELOG.md

Description

  • spec: queue the init prompt in MockSsh
  • spec: add procurve model unit test
  • procurve: fix snmp-server host secret redaction, redact local user password hashes (possibly fixes Hide SNMP secrets for Procurve #3774)
  • procurve: rstrip commented sections
  • models: replace slice and rstrip with cut_* and rstrip_lines refinement
  • docs: add @thanegill as a procurve model maintainer, note that some additional HPE Aruba switches use the procurve model
  • procurve: use clean :escape_codes and simplify the model

@ytti

ytti commented Jun 8, 2026

Copy link
Copy Markdown
Owner

You don't define rstrip_cfg method?

You probably intended something like:

comment cfg.each_line.map(&:rstrip).join("\n") + "\n"

But then we might ask, why not add this to :all. block?

@thanegill

Copy link
Copy Markdown
Contributor Author

You don't define rstrip_cfg method?

Oops, I thought that was defined in refinements. Added, copied from aosw.rb.

@thanegill

Copy link
Copy Markdown
Contributor Author

But then we might ask, why not add this to :all. block?

We could. But I'm not certain there is any training whitespace that is needed to restore the config accurately.

@ytti

ytti commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Maybe rather something like

def comment_clean str
  comment str.each_line.map(&:rstrip).join("\n") + "\n"
end

So that you're not repeating yourself that much.

Also, if you can, please provide spec/model/data files for testing purposes. I'm uncomfortable as this was submitted with non-working code, so I don't know if it works at all, or if it breaks for everyone.

thanegill added 5 commits June 9, 2026 16:47
MockSsh delivered the init prompt synchronously inside open_channel, before
its return value was assigned to SSH's @SES. A model that sends data in
response to the init prompt. e.g., Procurve answering "Press any key to
continue" - would hit a nil @SES. Queue the init prompt instead so it is
delivered on the first ssh.loop (in #login), by which time @SES is set.
Add a device-simulation unit test for the Procurve model (2930M-48G-PoE+,
WC.16.11.0028) plus a prompt test, captured with extra/device2yaml.rb.

To make the captured session replayable, Procurve sends Enter (not Space) for
the "Press any key" prompt, and its pre_logout is split into separate
logout/y/n sends, matching what device2yaml records line-by-line.
…ssword hashes

ProCurve emits `snmp-server host <ip> community "<name>" ...`, but the secret
rule matched `snmp-server host <ip> <community>` and so redacted the literal
word "community", leaving the community name exposed. Match the optional
`community` keyword and hide the name.

Also redact `password ... sha1 "<hash>"` local user password hashes (which may
wrap onto the next line in captured output), which were previously stored when
remove_secret was set. Both covered by a new secret test.
Strip trailing whitespace from the commented `show` command sections by
routing them through a new `clean_comment` helper (comment + rstrip_lines),
adding a `String#rstrip_lines` refinement. The regenerated test output shows
the trailing-whitespace removal.
@thanegill thanegill changed the title procurve: rstrip commented sections procurve: add model spec, fix secret redaction, simplify the model, rstrip comments Jun 9, 2026
@thanegill

Copy link
Copy Markdown
Contributor Author

Added the spec model data for tests. Found a few other bugs along the way. Let me know if this would be better suited as multiple PRs.

thanegill added 2 commits June 9, 2026 16:55
…dditional HPE Aruba switches use the procurve model
 - Replace the vt100 escape code stripping with :escape_codes
 - Since escape codes are now stripped before prompt detection, the
   prompt and expect's no longer need to account for them.
 - Use the cut_tail and reject_lines refinements.
@thanegill thanegill force-pushed the rstrip-procurve branch 2 times, most recently from d8aa439 to 16dc56a Compare June 10, 2026 03:34
…ecrets

The :secret rule only covered SNMP communities, sha1 password hashes,
and RADIUS/TACACS+ keys. With include-credentials (and
encrypt-credentials) the running-config can also carry SNMPv3 auth/priv
passwords, plaintext/sha256 and encrypted-password local passwords,
RADIUS/TACACS+ encrypted-key (resolves ytti#3774), key-chain key material,
802.1X supplicant secrets, SNTP authentication keys, the MACsec
pre-shared CAK, and the encrypt-credentials master
pre-shared-key. Redact all of them (drawn from the ArubaOS-Switch Access
Security Guide).

Add two synthetic fixtures, one per mutually-exclusive mode, covering
the cleartext/hashed and encrypted- forms respectively.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

2 participants