Skip to content

Conversation

devampkid
Copy link
Contributor

@devampkid devampkid commented Aug 21, 2025

#991

create a valid token here: https://huggingface.co/settings/tokens

@devampkid devampkid marked this pull request as ready for review August 21, 2025 20:40
@devampkid
Copy link
Contributor Author

devampkid commented Aug 27, 2025

Hello @erikvarga
I'm willing to write a more verbose validator for the validation since, from the validation endpoint response, we have three kinds of tokens: read, write, and fine-tuned tokens. An example response is like:

{"type":"user","id":"aaaaaaaa","name":"aaaaaaaa","fullname":"aaaaaaaa","isPro":false,"avatarUrl":"https://cdn-avatars.huggingface.co/v1/production/uploads/no-auth/aaaaaaaaaa.png","orgs":[],"auth":{"type":"access_token","accessToken":{"displayName":"test2","role":"fineGrained","createdAt":"2025-08-21T20:32:27.657Z","fineGrained":{"canReadGatedRepos":true,"global":[],"scoped":[{"entity":{"_id":"aaaaaaaa","type":"user","name":"aaaaaaaa"},"permissions":["inference.endpoints.infer.write","repo.content.read","repo.write","inference.serverless.write","inference.endpoints.write"]}]}}}}

The response tells us about the token type, permissions, full name, display name, ID, and ....
I'm asking if you want to contain this info in the results or not? (If this increases the bounty, I'm definitely willing to work more on the validator part!)

@erikvarga
Copy link
Collaborator

Hi @devampkid,

The info that would be useful for us is what kind of access a user can get with this specific token - So the permissions in this case like canReadGatedRepos. The rest (name, id, etc.) we'd prefer to leave out since those are PII that we might not be able to store in all our internal systems.

The panel will make the final decision on the exact payout but the more useful data we have available from validation the more likely it is that we'll pay the maximum reward amount for the category - see the rules page for the exact amounts (https://bughunters.google.com/about/rules/open-source/6436351477940224/osv-scalibr-patch-rewards-program-rules)

@devampkid
Copy link
Contributor Author

devampkid commented Sep 4, 2025

@erikvarga

I don't know where I can add the additional info, inside the Detector (the Detect Function) or inside the Validator(the Validate Function)

We want to add information we retrieve after Validation, not during the Detection(I mean the Detect function in the detector).

@erikvarga
Copy link
Collaborator

erikvarga commented Sep 5, 2025

@erikvarga

I don't know where I can add the additional info, inside the Detector (the Detect Function) or inside the Validator(the Validate Function)

We want to add information we retrieve after Validation, not during the Detection(I mean the Detect function in the detector).

Yeah, it should be added inside the Validator in that case.

I'm not quite sure if the current Validator interface allows you to modify the secret though (we might be passing it through as a value not a pointer). I'd suggest you give it a try and see if the secrets ends up getting the modifications when you run the SCALIBR CLI.

If not, you'll have to write your own Enricher plugin instead of reusing the interfaces from secrets/enricher.go
For that you'll need to implement the Enricher interface similar to how it's done in secrets/enricher.go, but only for your specific secret type validation. Then you can enable that enricher separately in enricher/enricherlist/list.go. LMK if you need any more pointers.

if err := ctx.Err(); err != nil {
return err
}
status, err := e.engine.Validate(ctx, s.Secret)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erikvarga I created a new enricher, but here if we use &s.Secret, we will get ValidationUnsupported status from here: https://github.com/devampkid/osv-scalibr/blob/12710d1d6a0a70882b66f71c9bcdc632e8e50e92/veles/validate.go#L120

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure yet how to continue sadly

Copy link
Collaborator

@erikvarga erikvarga Sep 8, 2025

Choose a reason for hiding this comment

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

I think you can skip using the veles validationengine for this enricher completely. Instead of calling engine.Validate in here you can implement the enrichment logic separately. Something like the following:

  • Loop through inv.Secrets
  • Look for ones of the type Huggingface
  • Call the API with them to fetch the additional information
  • Update the Secret with the additional information from the API

@erikvarga
Copy link
Collaborator

After some internal discussion we decided it might be better to keep validation separate from adding additional info to secrets.

Would you mind splitting the validation part and the fetching of the permissions field into two separate plugins? Essentially have two plugins that both query the huggingface endpoints for different reasons, one for validating the secret and the other for adding the permissions field.

The validator plugin can be a regular veles.Validator (mostly what you have in veles/secrets/huggingfaceapikey/validator.go) that is enabled in enricher/secrets/secrets.go.

The plugin that adds the permissions field can be a standalone Enricher plugin (i.e. what your enricher/huggingfacesecrets/huggingfacesecrets.go is now) that just populates the permissions field in the Secret but doesn't do any validation - this can be enabled separately in enricher/enricherlist/list.go.

Does that make sense? LMK if you have any questions

@devampkid
Copy link
Contributor Author

@erikvarga I created a new enricher for appending roles and scopes.

copybara-service bot pushed a commit that referenced this pull request Sep 19, 2025
@erikvarga
Copy link
Collaborator

Merged in 11df6d9

@erikvarga erikvarga closed this Sep 19, 2025
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