Strict ACL parsing without a regex

[ttfkam]

Description

Supersedes #2586 per maintainer comments.

Motivation: browsing through the code and saw a simple opportunity.

• Improve validation on ACL string: prevent "**", "*m", "z", "%", etc.

No new tests. Should only require regression testing.

Performance impact

Minor speed improvement for ACL parsing logic in aggregate due to one less regex.

Security impact

Excludes most invalid ACL input. (Doesn't stop duplicate ACL tokens.)

Checklist

• My code matches the project's code style and yarn lint:fix passes.
• I've added tests for the new feature, and yarn test passes.
• I have detailed the new feature in the relevant documentation.
• I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
• If this is a breaking change I've explained why.

[changeset-bot]

🦋 Changeset detected

Latest commit: 3772a8e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
pg-introspection	Patch
graphile-build-pg	Patch
pgl	Patch
graphile-utils	Patch
postgraphile	Patch
graphile	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[benjie]

This definitely looks more performant - a single loop over the string rather than at least 26! However, it's not as clearly correct as it was - the previous code was very obvious and this code is less so - so it will need some unit tests. Just simple assertions that a number of standard permissions strings with/without role, with/without grants parse correctly. Since pg-introspection doesn't seem to have tests already, please use node:test for them (or mocha/chai if you prefer).

In general we trust permission strings to be valid (we trust Postgres) so I'm not too concerned about malicious permission strings, however Postgres does add new permissions from time to time so it's worth ensuring that the code correctly handles (via an error) a new permission character.

I've included a number of TypeScript improvements, all minor.

[benjie]

Whilst implementing tests for this I discovered we had a bug in the way that we were handling roles containing " characters, and also that our permission string output was in a different order to Postgres'. Both of these are now resolved. Thanks!

[ttfkam]

Hey, sorry for dropping the ball here. I got caught up at work. Thank you for taking this past the finish line.