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

Invalid JSON config file when using new allowlist NSPRecord syntax #288

Closed
kyletsang opened this issue Dec 15, 2022 · 3 comments · Fixed by #289
Closed

Invalid JSON config file when using new allowlist NSPRecord syntax #288

kyletsang opened this issue Dec 15, 2022 · 3 comments · Fixed by #289
Assignees
Labels
bug Something isn't working

Comments

@kyletsang
Copy link
Contributor

Getting this error

Invalid JSON config file: .audit-ci.json

It appears to be due to yargs performing some validation on the allowlist structure. I'll take a closer look when I get a chance, but maybe you have an idea on what to do here @quinnturner?

@quinnturner quinnturner added the bug Something isn't working label Dec 15, 2022
@quinnturner quinnturner self-assigned this Dec 15, 2022
@quinnturner
Copy link
Member

From what I can tell, it seems that Yargs does not support object arrays. In retrospect, I can imagine it is difficult to pass an array of objects through the CLI, so it may not be implemented.

I am not sure yet how I'd like to proceed. I firmly push toward using configuration files because the allowlist makes this a helpful library nowadays. From what I recall, most package managers now natively support audit levels. Yarn 3.3.0 or 3.3.1 ish will support allowlisting using the NPM identifier (which is less valuable than the GitHub identifier).

With that in mind, one option is to migrate towards another config-focused library and entirely remove CLI argument support. I wouldn't say that is ideal as it's a breaking change especially since a considerable population of open-source projects use it for solely auditing levels and not the allowlisting.

Open to ideas!

@kyletsang
Copy link
Contributor Author

I dug a bit deeper and it seems like yargs doesn't like the output of the object array when parsed with jju. It does accept the one from JSON.parse.

Interestingly, if we pass the null_prototype: false option into jju's parse function, then it starts working. I tested this on my project and it works. I "think" this should be safe?

I'll open a PR so you can view the diff

@quinnturner
Copy link
Member

I came to the exact same conclusion 😄 reviewing now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants