Fix potential error revoking privileges in postgres secret engine #28519
Conversation
| @@ -529,7 +529,7 @@ func (p *PostgreSQL) defaultDeleteUser(ctx context.Context, username string) err | |||
| } | |||
| revocationStmts = append(revocationStmts, fmt.Sprintf( | |||
| `REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA %s FROM %s;`, | |||
| (schema), | |||
| dbutil.QuoteIdentifier(schema), | |||
There was a problem hiding this comment.
I am not so sure this is a straightforward change. Is this backwards compatible? It doesn't appear to be so based on http://www.postgresql.org/docs/9.2/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
Quoting an identifier also makes it case-sensitive, whereas unquoted names are always folded to lower case.
So we might break users that depend on case-insensitivity.
There was a problem hiding this comment.
hmm, it looks like we already quote the schema name in the next revoke call here: https://github.com/hashicorp/vault/blame/2db2a9fb5d890b213a2c05aa5698c63560399774/plugins/database/postgresql/postgresql.go#L537
There was a problem hiding this comment.
Yeah it used to be quoted but it was (unintentionally I guess) dropped in this PR: #15343
There was a problem hiding this comment.
Thanks! Do you mind adding a changelog entry?
changelog/28519.txt
database/postgresql: Fix potential error revoking privileges in postgresql database secrets engine when a schema contains special characters
|
Would it be possible to have this backported to 1.16.x? |
Description
Adds a missing call to quote the schema name when revoking privileges. If there's a dash in the name you will get a syntax error otherwise.
TODO only if you're a HashiCorp employee
to N, N-1, and N-2, using the
backport/ent/x.x.x+entlabels. If this PR is in the CE repo, you should only backport to N, using thebackport/x.x.xlabel, not the enterprise labels.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.