Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(template): incorrect JSON marshaling for some fields #7464

Closed
afdesk opened this issue Sep 9, 2024 Discussed in #7448 · 15 comments · Fixed by #7483
Closed

bug(template): incorrect JSON marshaling for some fields #7464

afdesk opened this issue Sep 9, 2024 Discussed in #7448 · 15 comments · Fixed by #7483
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@afdesk
Copy link
Contributor

afdesk commented Sep 9, 2024

Problem

There are cases when a custom template doesn't covert some result fields into JSON.

Trivy doesn't call custom marshal functions:
Status doesn't become string, PURL for PkgIdentifier is empty.

Reproduction steps:

$ trivy image --format json --output report.json alpine:3.20.2
$ trivy convert --format template --template '{{ range . }}{{ toPrettyJson . }}{{ end }}' report.json | grep Status | head -n 1
      "Status": "fixed",

$ trivy convert --format template --template '{{ range . }}{{ range .Vulnerabilities }}{{ toPrettyJson . }}{{ end }}{{ end }}' report.json | grep Status | head -n 1
  "Status": 3,

Reason

toPrettyJson from sprig uses a standart JSON marshal function - json.MarshalIndent:
https://github.com/Masterminds/sprig/blob/e708470d529a10ac1a3f02ab6fdd339b65958372/defaults.go#L122-L124

Go JSON uses relection and reflect library marks some values as addressable:

// reports whether the value's address can be obtained with [Value.Addr].
// Such values are called addressable. A value is addressable if it is
// an element of a slice, an element of an addressable array,
// a field of an addressable struct, or the result of dereferencing a pointer.
// If CanAddr returns false, calling [Value.Addr] will panic.

https://cs.opensource.google/go/go/+/refs/tags/go1.23.1:src/reflect/value.go;drc=5fee159bc2e60736ce967560ee5be738fe5d5bd2;l=336-343

Go uses different encoders for addressable and non-addressable values:
https://cs.opensource.google/go/go/+/refs/tags/go1.23.1:src/encoding/json/encode.go;l=900-906

For non-addressable values encoder doesn't call marshall function, so Status and PURL aren't converted.

Solutions

  1. We just add into the docs this case, and don't do anything.

  2. We convert non-addressable fields with Marshal function into pointers. It makes these ones addressable.
    Note: it'll take some efforts. For example, Status struct describes in trivy-db.

Related PRs

this issue also affects on #7463.

Discussed in #7448

@afdesk afdesk added the kind/bug Categorizes issue or PR as related to a bug. label Sep 9, 2024
@afdesk afdesk self-assigned this Sep 9, 2024
@afdesk
Copy link
Contributor Author

afdesk commented Sep 9, 2024

@knqyf263 @DmitriyLewen wdyt?

@DmitriyLewen
Copy link
Contributor

We just add into the docs this case, and don't do anything.

We need to think about template support. @itaysk said: the contrib templates will be removed at some point in favor of community maintained plugins.

  • What about user templates?
  • Will we support templates? Or will we ask users to migrate to plugins and deprecate the template format?
    If so, description of this nuance in the docs looks good.

We convert non-addressable fields with Marshal function into pointers. It makes these ones addressable.
Note: it'll take some efforts. For example, Status struct describes in trivy-db.

I see 2 possible ways:

@knqyf263
Copy link
Collaborator

For non-addressable values encoder doesn't call marshall function, so Status and PURL aren't converted.

I'm curious why Status is non-addressable.

@afdesk
Copy link
Contributor Author

afdesk commented Sep 10, 2024

I'm curious why Status is non-addressable.

Status type is int. It's not a pointer.
https://github.com/aquasecurity/trivy-db/blob/42b851ca3d8ce1b1b49cb012d228a7ee738496b6/pkg/types/status.go#L5

@knqyf263
Copy link
Collaborator

But json.Marshal and json.MarshalIndent call the custom method. Why isn't it called in this case?
https://go.dev/play/p/b7okF1jln0N

@afdesk
Copy link
Contributor Author

afdesk commented Sep 10, 2024

But json.Marshal and json.MarshalIndent call the custom method. Why isn't it called in this case? https://go.dev/play/p/b7okF1jln0N

json doesn't do dereferencing in this case...

func (s *Status) MarshalJSON() ([]byte, error) {
	return json.Marshal("hello")
}

https://go.dev/play/p/KXldzyqSHTq

@afdesk
Copy link
Contributor Author

afdesk commented Sep 10, 2024

it seems if json package can't find MarshalJSON for type, this type will mark as non-addressable.

I can check it, but the solutions are the same: change Status and PkgIdentifier, or skip it.

@knqyf263
Copy link
Collaborator

This is really interesting...
https://go.dev/play/p/qolkSCB2lvn

@knqyf263
Copy link
Collaborator

I opened a PR.
aquasecurity/trivy-db#436

In your example, toPrettyJson is called on each DetectedVulnerability, and that's the reason why Status is not marshalled correctly.

It fixed the problem in my environment.

./trivy convert --format template --template '{{ range . }}{{ range .Vulnerabilities }}{{ toPrettyJson . }}{{ end }}{{ end }}' report.json | grep Status | head -n 1
  "Status": "fixed",

@knqyf263
Copy link
Collaborator

Also, I'll fix

func (id *PkgIdentifier) MarshalJSON() ([]byte, error) {

@afdesk
Copy link
Contributor Author

afdesk commented Sep 10, 2024

I opened a PR. aquasecurity/trivy-db#436

In your example, toPrettyJson is called on each DetectedVulnerability, and that's the reason why Status is not marshalled correctly.

It fixed the problem in my environment.

oh, you found out the root issue. that's cool!

@eissko
Copy link

eissko commented Sep 10, 2024

can be this related also to this bug? suddenly after few months the trivy 0.53.0 started to fail on umarshaling the filed of Ubuntu scan ending with following error:

Fatal error image scan error: scan error: scan failed: scan failed: failed to detect vulnerabilities: unable to scan OS packages: failed vulnerability detection of OS packages: failed detection: failed to get Ubuntu advisories: failed to get Ubuntu advisories: failed to unmarshal advisory JSON: json: cannot unmarshal string into Go struct field dbAdvisory.Status of type int

thank you for any help,
Peter

@itaysk
Copy link
Contributor

itaysk commented Sep 10, 2024

What about user templates?
Will we support templates? Or will we ask users to migrate to plugins and deprecate the template format?

I think we can continue to support template functionality, but not maintain templates in Trivy's tree.

@Altourus
Copy link

Altourus commented Sep 10, 2024

can be this related also to this bug? suddenly after few months the trivy 0.53.0 started to fail on umarshaling the filed of Ubuntu scan ending with following error:

Fatal error image scan error: scan error: scan failed: scan failed: failed to detect vulnerabilities: unable to scan OS packages: failed vulnerability detection of OS packages: failed detection: failed to get Ubuntu advisories: failed to get Ubuntu advisories: failed to unmarshal advisory JSON: json: cannot unmarshal string into Go struct field dbAdvisory.Status of type int

thank you for any help, Peter

I'm also seeing this happen to my build agents now starting within the last few hours

@knqyf263
Copy link
Collaborator

knqyf263 commented Sep 10, 2024

@eissko @Altourus No, it's not relevant and it was fixed.
#7481

If the problem still persists, please try trivy clean --vuln-db.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
6 participants