Skip to content

Rename Resource/ResourceGroup via PutAsync + EntityPropertyWrite for safe hidden-field writes - #22

Open
davidnmbond wants to merge 2 commits into
mainfrom
feature/rename-and-hidden-field-writes
Open

Rename Resource/ResourceGroup via PutAsync + EntityPropertyWrite for safe hidden-field writes#22
davidnmbond wants to merge 2 commits into
mainfrom
feature/rename-and-hidden-field-writes

Conversation

@davidnmbond

Copy link
Copy Markdown
Contributor

Why

Surfaced while renaming ~294 LogicMonitor ping-check resources (PS-3019). Two things needed proving/adding:

  1. Rename works via PutAsync<T>. Resource and ResourceGroup are both IdentifiedItem, IHasEndpoint, so PutAsync already supports them. This adds integration tests that prove it, and captures a real footgun: on uptimepingcheck devices a rename must set both Name and DisplayName (a DisplayName-only change silently no-ops), while the ping target (Host) is preserved.

  2. Safe hidden-field writes. LogicMonitor masks secret custom properties (snmp.community, *.pass, *.key) as ******** on read, so round-tripping a whole object via PUT would write the mask back and clobber the real value. This adds EntityPropertyWrite — the admin-gated, config-as-code shape:

    [ { "type": "resourceGroup", "id": 1234, "name": "snmp.community", "value": "public" } ]

    plus SetCustomPropertyAsync(EntityPropertyWrite, …) / SetCustomPropertiesAsync(…), which write one field at a time and never send the mask.

Changes

  • New: EntityPropertyWrite, EntityPropertyWriteTargetType (LogicMonitor.Api/Resources).
  • New: LogicMonitorClient.SetCustomPropertyAsync(EntityPropertyWrite, SetPropertyMode, ct) and SetCustomPropertiesAsync(IEnumerable<EntityPropertyWrite>, …) — route to the existing per-field property setters.
  • Tests (unit, portal-free): EntityPropertyWriteTests — config deserialisation + endpoint mapping (4 tests, passing).
  • Tests (integration, live portal): ResourceRenameTests — PutAsync renames a ResourceGroup; PutAsync renames an uptime ping check (Name+DisplayName, Host preserved, skips if Uptime not enabled); hidden field written via EntityPropertyWrite.

Notes

  • Full solution builds clean (warnings-as-errors); unit tests pass. Integration tests require a live portal (creds via user secrets) and will run in CI.

🤖 Generated with Claude Code

…afe hidden-field writes

PutAsync<T> already supports Resource and ResourceGroup (both IdentifiedItem +
IHasEndpoint), so rename works via a full-object PUT. Add integration tests
proving it, including that a uptimepingcheck rename must set BOTH Name and
DisplayName (a DisplayName-only change silently no-ops on those devices) while
the ping target Host is preserved.

Because LogicMonitor masks secret custom properties (snmp.community, *.pass,
*.key) as ******** on read, round-tripping a whole object would write the mask
back and clobber the real value. Add EntityPropertyWrite (the admin-gated,
config-as-code shape { type, id, name, value }) plus SetCustomPropertyAsync /
SetCustomPropertiesAsync, which write one field at a time and never send the
mask. Portal-free unit tests cover the config deserialisation and endpoint
mapping; an integration test writes snmp.community via EntityPropertyWrite.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 1 high · 1 medium

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category Results
Security 1 high
Complexity 1 medium

View in Codacy

🟢 Metrics 18 complexity

Metric Results
Complexity 18

View in Codacy

AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

… Resource/ResourceGroup secrets

Ran a live experiment on the panoramicdata portal: set a hidden custom property
(test.pass), did a full-object GET->PUT round-trip (sending back the masked
********), then read the true stored value via a reveal PropertySource (with a
fresh-run marker to rule out staleness). The value was preserved - LogicMonitor
treats the ******** mask as "unchanged" on write. So PutAsync-based rename is
secret-safe; the earlier "PUT clobbers" rationale was wrong for device/group
properties (it originated from a NetScan credentials incident, a different path).

EntityPropertyWrite remains the admin-gated, config-as-code way to set/rotate
hidden fields (you supply the real value). Docs/comments corrected accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@davidnmbond

Copy link
Copy Markdown
Contributor Author

Empirical result: full PUT does not clobber Resource/ResourceGroup secrets

Ran the clobber experiment on a live portal to settle the masked-secret concern:

  1. Created a throwaway device with test.pass=SuperSecret123 (+ unhide.properties=test.pass).
  2. Deployed a reveal PropertySource (AppliesTo unhide.properties) that echoes nominated hidden props to auto.* fingerprints (md5/len/head) — letting us see past the ******** API mask.
  3. Baseline: auto.unhide.result.0 = test.pass|len=14|md5=28db9e12… ✅ (collector script gets the real value).
  4. Did a full-object GET → PUT round-trip (body carried test.pass="********", ?ignoreReference=true — exactly what PutAsync<Resource> sends).
  5. Planted a fresh-run marker (new prop via a per-field write) to prove the PropertySource re-ran after the PUT, defeating stale-value ambiguity.
  6. Fresh read: test.pass still len=14 md5=28db9e12… head=SuSuperSecret123 preserved, not clobbered.

Conclusion: LogicMonitor treats the ******** mask as leave unchanged on a device/group PUT, so PutAsync-based rename is secret-safe. Docs/comments in this PR corrected to say so (the earlier "PUT clobbers" wording came from a NetScan credentials.custom incident — a different path that is not covered here and still warrants caution).

EntityPropertyWrite stays as the admin-gated, config-as-code way to set/rotate hidden fields (caller supplies the real value). Test resource + PropertySource were cleaned up after the run.

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