Skip to content

Add debug logging for processRequest, IDScan and documentExpiryRejection - #72

Closed
Didr wants to merge 4 commits into
mainfrom
feat/processrequest-debuglog
Closed

Didr wants to merge 4 commits into
mainfrom
feat/processrequest-debuglog

Conversation

@Didr

@Didr Didr commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Added this while debugging #70 and #71.

Didr added 2 commits October 7, 2026 11:06
We want more information on what data was actually retrieved from the credential.
@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Didr

Didr commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot go bananas on this PR. Codecov is also complaining about the "documentDataForLog" function missing a test, please add that.

Co-authored-by: Didr <9605751+Didr@users.noreply.github.com>

Copilot AI commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@copilot go bananas on this PR. Codecov is also complaining about the "documentDataForLog" function missing a test, please add that.

Added table-driven coverage for portrait redaction, the empty-portrait case, and preservation of document fields. internal/apiv1 passes; documentDataForLog is at 100% coverage. Commit: 429571c.

@sonarqubecloud

sonarqubecloud Bot commented Oct 7, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Debug logs expose personal identity and expiry data contrary to the repository’s privacy policy.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Adds debug diagnostics for document parsing in ID scan and process-request flows to investigate issues #70 and #71.

Changes:

  • Logs OCR document data with portrait omission.
  • Adds expiry-check diagnostic fields.
  • Updates and adds related tests.
File Description
internal/​apiv1/​client.go Adds document and expiry debug logging.
internal/​apiv1/​liveness_expiry_test.go Updates expiry tests and tests portrait omission.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/apiv1/client.go Outdated

// documentDataForLog is the OCR result written at debug level. The portrait
// is a base64 face image, so only its length is included.
func documentDataForLog(doc facetec.DocumentData) facetec.DocumentData {
@Didr

Didr commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

@copilot would you be so kind to take a look again after #75 was merged?

Co-authored-by: Didr <9605751+Didr@users.noreply.github.com>

Copilot AI commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

@copilot would you be so kind to take a look again after #75 was merged?

Addressed the privacy review finding in 33c27b8. documentDataForLog now returns an allowlisted summary: the document type plus per-field presence and length, with no values. The expiry-check log no longer includes the raw expiry date; it logs the date's length, whether it parsed, days until expiry, and the result. The test now asserts that none of the identity values appear in the logged output. internal/apiv1 tests pass.

@Didr

Didr commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Already implemented in #75

@Didr Didr closed this Oct 12, 2026
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.

3 participants