Skip to content

Commit f9abc1d

Browse files
committed
BREAKING CHANGE: create superuser
The 'setup-postgres' action used to create a superuser that hasn't been exposed to users via 'connection-uri' output. The superuser has been named after GitHub Action's system user (i.e. 'runner'), had no password and could have been used via PostgreSQL client applications [1] or when using manually constructed connection URI with no user set. The user set via action's input parameters used to be unprivileged with escalated permissions to create databases on-demand. I don't remember why I made things this way, maybe I got confused somewhere along the way, but I don't think having both private superuser and public unprivileged user is a good idea. It's quite common in tests to dynamically create databases and/or users for applications under test, thus superuser permissions are required. This patch removes a private superuser named after the GitHub Action's system user (i.e. 'runner') in favor of granting superuser permissions to a user set via action's input parameters. Those who explicitly relied on 'runner' user might got affected as the user WON'T exist anymore. [1] https://www.postgresql.org/docs/15/reference-client.html
1 parent 3973215 commit f9abc1d

File tree

4 files changed

+145
-39
lines changed

4 files changed

+145
-39
lines changed

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ jobs:
2626

2727
- name: Run tests
2828
run: |
29-
python3 -m pip install --upgrade pip pytest psycopg
29+
python3 -m pip install --upgrade pip pytest psycopg furl
3030
python3 -m pytest -vv test_action.py
3131
env:
3232
CONNECTION_URI: ${{ steps.postgres.outputs.connection-uri }}
@@ -55,7 +55,7 @@ jobs:
5555

5656
- name: Run tests
5757
run: |
58-
python3 -m pip install --upgrade pip pytest psycopg
58+
python3 -m pip install --upgrade pip pytest psycopg furl
5959
python3 -m pytest -vv test_action.py
6060
env:
6161
CONNECTION_URI: ${{ steps.postgres.outputs.connection-uri }}

README.md

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,19 @@
33
This action sets up a PostgreSQL server for the rest of the job. Here are some
44
key features:
55

6-
* Runs on Linux, macOS and Windows runners.
7-
* Adds PostgreSQL [binaries][1] (e.g. `psql`) to `PATH`.
8-
* Uses PostgreSQL installed in [GitHub Actions Virtual Environments][2].
9-
* [Easy to check][3] that IT DOES NOT contain malicious code.
6+
* Runs on Linux, macOS and Windows action runners.
7+
* Adds PostgreSQL [client applications][1] to `PATH`.
8+
* Uses PostgreSQL binaries baked into [GitHub Actions Runner Images][2].
9+
* Easy [to prove][3] that it DOES NOT contain malicious code.
1010

1111
[1]: https://www.postgresql.org/docs/current/reference-client.html
12-
[2]: https://github.com/actions/virtual-environments
12+
[2]: https://github.com/actions/runner-images
1313
[3]: action.yml
1414

1515
## Usage
1616

17+
#### Connection parameters
18+
1719
| Key | Value |
1820
|----------|-----------------------------------------------------|
1921
| URI | `postgresql://postgres:postgres@localhost/postgres` |
@@ -23,6 +25,13 @@ key features:
2325
| Password | `postgres` |
2426
| Database | `postgres` |
2527

28+
#### User permissions
29+
30+
| Key | Value |
31+
|-------------|-------|
32+
| usesuper | true |
33+
| usecreatedb | true |
34+
2635
#### Basic
2736

2837
```yaml
@@ -47,6 +56,13 @@ steps:
4756
DATABASE_URI: ${{ steps.postgres.outputs.connection-uri }}
4857
```
4958
59+
## Rationale
60+
61+
At the time of developing there were no GitHub Actions on the marketplace to
62+
setup a PostgreSQL server on Linux, Windows and macOS action runners. Most
63+
solutions suggest using Docker which is not available on macOS and Windows
64+
runners.
65+
5066
## License
5167
5268
The scripts and documentation in this project are released under the

action.yml

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,42 +38,56 @@ runs:
3838
fi
3939
shell: bash
4040

41-
4241
- name: Setup and start PostgreSQL
4342
run: |
4443
export PGDATA="$RUNNER_TEMP/pgdata"
45-
pg_ctl init --options="--encoding=UTF-8 --locale=en_US.UTF-8"
4644
47-
# Forbid creating unix sockets since they are created by default in the
48-
# directory we don't have permissions to.
45+
# There are couple of reasons why we need to create a new PostgreSQL
46+
# database cluster. First and foremost, we have to create a superuser
47+
# with provided credentials. Second, we want the PostgreSQL client
48+
# applications [1] to be available for execution without
49+
# run-from-another-user dances. Third, we want to make sure that
50+
# settings are the same between operating systems and aren't changed by
51+
# package vendors.
52+
#
53+
# [1] https://www.postgresql.org/docs/15/reference-client.html
54+
initdb \
55+
--username="${{ inputs.username }}" \
56+
--encoding="UTF-8" \
57+
--locale="en_US.UTF-8" \
58+
--no-instructions
59+
60+
# Do not create unix sockets since they are created by default in the
61+
# directory we have no permissions to (owned by system postgres user).
4962
echo "unix_socket_directories = ''" >> "$PGDATA/postgresql.conf"
5063
echo "port = ${{ inputs.port }}" >> "$PGDATA/postgresql.conf"
5164
pg_ctl start
5265
53-
# Both PGHOST and PGUSER are used by PostgreSQL tooling such as 'psql'
54-
# or 'createuser'. Since PostgreSQL data has been resetup, we cannot
55-
# rely on defaults anymore.
66+
# Set environment variables for PostgreSQL client applications [1] such
67+
# as 'psql' or 'createuser'.
5668
#
57-
# PGHOST is required for Linux and macOS since they default to unix
58-
# sockets, and we have turned them off.
69+
# PGHOST is required for Linux/macOS because we turned off unix sockets
70+
# and they use them by default.
5971
#
60-
# PGUSER is required for Windows since default the tooling's default
61-
# user is 'postgres', while 'pg_ctl init' creates one with the name of
62-
# the current user.
72+
# PGPORT, PGUSER and PGDATABASE are required because they could be
73+
# parametrized via action input parameters.
74+
#
75+
# [1] https://www.postgresql.org/docs/15/reference-client.html
6376
echo "PGHOST=localhost" >> $GITHUB_ENV
64-
echo "PGUSER=${USER:-$USERNAME}" >> $GITHUB_ENV
6577
echo "PGPORT=${{ inputs.port }}" >> $GITHUB_ENV
78+
echo "PGUSER=${{ inputs.username }}" >> $GITHUB_ENV
79+
echo "PGDATABASE=${{ inputs.database }}" >> $GITHUB_ENV
6680
shell: bash
6781

68-
- name: Setup PostgreSQL user and database
82+
- name: Setup PostgreSQL database
6983
run: |
70-
createuser --createdb ${{ inputs.username }}
71-
72-
if [ "${{ inputs.database}}" != "postgres" ]; then
73-
createdb -O ${{ inputs.username }} ${{ inputs.database }}
84+
# The 'postgres' database is a pre-created database meant for use by
85+
# users, utilities and third party applications. There's no way to
86+
# parametrize the name, so all we can do is to avoid creating a
87+
# database if provided name is 'postgres'.
88+
if [ "${{ inputs.database }}" != "postgres" ]; then
89+
createdb -O "${{ inputs.username }}" "${{ inputs.database }}"
7490
fi
75-
76-
psql -c "ALTER USER ${{ inputs.username }} PASSWORD '${{ inputs.password }}';" ${{ inputs.database }}
7791
shell: bash
7892

7993
- name: Expose connection URI

test_action.py

Lines changed: 88 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,40 @@
1-
import typing as t
1+
import locale
22
import os
3+
import subprocess
4+
import typing as t
35

46
import psycopg
7+
import furl
58
import pytest
69

710

11+
ConnectionFactory = t.Callable[[str], psycopg.Connection]
12+
13+
14+
@pytest.fixture(scope="function")
15+
def connection_uri() -> str:
16+
"""Read and return connection URI from environment."""
17+
18+
connection_uri = os.getenv("CONNECTION_URI")
19+
if connection_uri is None:
20+
pytest.fail("CONNECTION_URI: environment variable is not set")
21+
return connection_uri
22+
23+
824
@pytest.fixture(scope="function")
9-
def connection_factory() -> t.Callable[[str], psycopg.Connection]:
25+
def connection_factory() -> ConnectionFactory:
26+
"""Return 'psycopg.Connection' factory."""
27+
1028
def factory(connection_uri: str) -> psycopg.Connection:
1129
return psycopg.connect(connection_uri)
1230
return factory
1331

1432

1533
@pytest.fixture(scope="function")
16-
def connection(connection_factory) -> psycopg.Connection:
17-
return connection_factory(os.getenv("CONNECTION_URI"))
34+
def connection(connection_uri: str, connection_factory: ConnectionFactory) -> psycopg.Connection:
35+
"""Return 'psycopg.Connection' for connection URI set in environment."""
36+
37+
return connection_factory(connection_uri)
1838

1939

2040
def test_connection_uri():
@@ -34,12 +54,15 @@ def test_server_encoding(connection: psycopg.Connection):
3454
def test_locale(connection: psycopg.Connection):
3555
"""Test that PostgreSQL's locale is 'en_US.UTF-8'."""
3656

37-
assert connection.execute("SHOW LC_COLLATE").fetchone()[0] == "en_US.UTF-8"
38-
assert connection.execute("SHOW LC_CTYPE").fetchone()[0] == "en_US.UTF-8"
57+
lc_collate = connection.execute("SHOW LC_COLLATE").fetchone()[0]
58+
lc_ctype = connection.execute("SHOW LC_CTYPE").fetchone()[0]
59+
60+
assert locale.normalize(lc_collate) == "en_US.UTF-8"
61+
assert locale.normalize(lc_ctype) == "en_US.UTF-8"
3962

4063

4164
def test_user_permissions(connection: psycopg.Connection):
42-
"""Test that a user can create databases but is not a superuser."""
65+
"""Test that a user has super/createdb permissions."""
4366

4467
with connection:
4568
record = connection \
@@ -49,7 +72,7 @@ def test_user_permissions(connection: psycopg.Connection):
4972

5073
usecreatedb, usesuper = record
5174
assert usecreatedb
52-
assert not usesuper
75+
assert usesuper
5376

5477

5578
def test_user_create_insert_select(connection: psycopg.Connection):
@@ -82,12 +105,65 @@ def test_user_create_insert_non_ascii(connection: psycopg.Connection):
82105

83106

84107
def test_user_create_drop_database(connection: psycopg.Connection):
85-
"""Test that a user has no permissions to create databases."""
108+
"""Test that a user has permissions to create databases."""
86109

87110
# CREATE/DROP DATABASE statements don't work within transactions, and with
88111
# autocommit disabled transactions are created by psycopg automatically.
89112
connection.autocommit = True
90113

91-
database_name = "foobar42"
92-
connection.execute(f"CREATE DATABASE {database_name}")
93-
connection.execute(f"DROP DATABASE {database_name}")
114+
database = "databas3"
115+
connection.execute(f"CREATE DATABASE {database}")
116+
connection.execute(f"DROP DATABASE {database}")
117+
118+
119+
def test_user_create_drop_user(
120+
connection: psycopg.Connection,
121+
connection_factory: ConnectionFactory,
122+
connection_uri: str
123+
):
124+
"""Test that a user has permissions to create users."""
125+
126+
# CREATE/DROP USER statements don't work within transactions, and with
127+
# autocommit disabled transactions are created by psycopg automatically.
128+
connection.autocommit = True
129+
130+
username = "us3rname"
131+
password = "passw0rd"
132+
database = "databas3"
133+
134+
connection.execute(f"CREATE USER {username} WITH PASSWORD '{password}'")
135+
connection.execute(f"CREATE DATABASE {database} WITH OWNER '{username}'")
136+
137+
try:
138+
# Smoke test that created user can successfully log-in and execute
139+
# queries for its own database.
140+
connection_uri = furl.furl(
141+
connection_uri, username=username, password=password, path=database).url
142+
test_user_create_insert_select(connection_factory(connection_uri))
143+
144+
finally:
145+
connection.execute(f"DROP DATABASE {database}")
146+
connection.execute(f"DROP USER {username}")
147+
148+
149+
def test_client_applications(connection_uri, connection_factory):
150+
"""Test that PostgreSQL client applications can be used."""
151+
152+
username = "us3rname"
153+
password = "passw0rd"
154+
database = "databas3"
155+
156+
subprocess.check_call(["createuser", username])
157+
subprocess.check_call(["createdb", "--owner", username, database])
158+
subprocess.check_call(["psql", "-c", f"ALTER USER {username} WITH PASSWORD '{password}'"])
159+
160+
try:
161+
# Smoke test that created user can successfully log-in and execute
162+
# queries for its own database.
163+
connection_uri = furl.furl(
164+
connection_uri, username=username, password=password, path=database).url
165+
test_user_create_insert_select(connection_factory(connection_uri))
166+
167+
finally:
168+
subprocess.check_call(["dropdb", database])
169+
subprocess.check_call(["dropuser", username])

0 commit comments

Comments
 (0)