Skip to content

Reports: a value masked by hand is no identifier - #536

Merged
openipc-ai merged 1 commit into
masterfrom
redact-masked-secrets
Oct 30, 2026
Merged

openipc-ai merged 1 commit into
masterfrom
redact-masked-secrets

Conversation

@openipc-ai

Copy link
Copy Markdown
Collaborator

Follow-up to #535. Running openipc reports redact --dry-run in production showed that r-7tb674nu's sender had replaced their boot log with a hand-masked copy, "CloudCryNum" : "#######". Redact took the mask for the value and would have replaced every ####### in the log: 59 lines of banners.

A value must now contain a letter or digit before it counts as an identifier. The real values in the same log (the serial in its SN: line, DevInfo's Data and Key) are still caught. Covered by TestAValueMaskedByHandIsLeftAlone.

The six other reports the dry run found were redacted in production with #535. This one waits for the fix.

r-7tb674nu's sender masked CloudCryNum as "#######" in a boot log
sent again; Redact took the mask for the value and would have replaced
all 59 runs of #s in the log. A value with no letter or digit is left
alone.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Avoid redacting hand-masked values in report logs

🐞 Bug fix 🧪 Tests 🕐 Less than 10 minutes

Grey Divider

AI Description

• Reject text-extracted values without letters or digits, preventing masks from redacting unrelated
 log content.
• Test that hand-masked values remain intact while a real DevInfo key is redacted.
Diagram

graph TD
  Input["Report text"] --> Extract["Text matches"] --> Check{"Letter or digit?"} -->|Yes| IDs["Identifiers"] --> Redact["Keyed redaction"]
  Check -->|No| Preserve["Keep mask intact"]
Loading
High-Level Assessment

Keep the guard in the shared text-identifier collector: it covers all text-matched value types without changing their individual extraction patterns. Tightening each pattern separately would duplicate the rule.

Files changed (2) +16 / -1

Bug fix (1) +6 / -1
parse.goExclude hand-masked text values from identifiers +6/-1

Exclude hand-masked text values from identifiers

• The shared text-identifier collector now rejects values containing no ASCII letter or digit. This prevents punctuation-only masks from triggering replacements throughout a log.

service/internal/reports/parse.go

Tests (1) +10 / -0
parse_test.goCover hand-masked values alongside a real secret +10/-0

Cover hand-masked values alongside a real secret

• Adds a regression test confirming that hash and asterisk masks remain unchanged, including repeated hash banners, while a numeric DevInfo key is redacted.

service/internal/reports/parse_test.go

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Punctuation-only secrets remain public 📘 Rule violation ⛨ Security
Description
textIdentifiers uses an ASCII-only alphanumeric guard that discards captured cloud_secret values
containing no ASCII letter or digit, including punctuation-only and Unicode-only values, before
Redact can replace them. When the broad CloudCryNum or DevInfo.Data patterns capture such
values, they remain in public report copies and redacted crash text.
Code

service/internal/reports/parse.go[421]

+		if (name == "cloud_id" && !strings.ContainsAny(v, "0123456789")) || !alnum.MatchString(v) || zeroish(v) {
Evidence
The CloudCryNum and DevInfo patterns accept values of six or more non-quote characters
regardless of whether those characters are punctuation or Unicode letters or digits. The new guard
rejects captures without an ASCII letter or digit before adding them to the identifier list, and
report preparation and crash ingestion use Redact output for public copies, allowing those
rejected values to remain.

CLAUDE.md: Protect Public Reports and Crash Data: CLAUDE.md: Protect Public Reports and Crash Data
service/internal/reports/parse.go[398-405]
service/internal/reports/parse.go[417-422]
service/internal/reports/parse.go[333-345]
service/internal/reports/handler.go[430-455]
service/internal/crashes/api.go[136-143]
service/internal/reports/parse.go[400-403]
service/internal/reports/parse.go[456-459]
service/internal/reports/parse.go[335-347]
service/internal/reports/handler.go[451-457]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new ASCII-only alphanumeric guard suppresses punctuation-only and Unicode-only secrets captured by the broad `CloudCryNum` and `DevInfo` patterns, leaving them in public report and crash copies. Hand-applied masks still need to remain unchanged.
## Fix Focus Areas
- service/internal/reports/parse.go[398-405]
- service/internal/reports/parse.go[420-422]
- service/internal/reports/parse.go[456-459]
- service/internal/reports/parse_test.go[293-302]
## Recommended Fix
Distinguish known hand-applied masks from valid captured secrets without globally dropping punctuation-only values, either by constraining the affected patterns to real identifier formats or by handling masks specifically. Ensure the guard recognizes Unicode letters and digits rather than only `[A-Za-z0-9]`, and add regression tests showing that hand masks remain unchanged while punctuation-only and Unicode-only sensitive values are redacted.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@openipc-ai

Copy link
Copy Markdown
Collaborator Author

On the one finding, punctuation-only secrets remain public: this is deliberate, not a gap. The values these patterns capture have fixed alphabets. Xiongmai's CloudCryNum is alphanumeric (o0pLR5yg-shaped), DevInfo's Data is base64 (A-Z, a-z, 0-9, +/=) and its Key is digits. A real value always has an ASCII letter or digit. The only captures without one are masks a sender typed by hand (#######, *********), which hide nothing and which, as matches, would replace every run of # in the log. The cloud IDs keep their stricter rule: they need a digit.

@openipc-ai
openipc-ai merged commit 1f0d224 into master Oct 30, 2026
2 checks passed
@openipc-ai
openipc-ai deleted the redact-masked-secrets branch October 30, 2026 13:27
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