140 lines
6.0 KiB
Markdown
140 lines
6.0 KiB
Markdown
# Task 1 Report
|
|
|
|
## Status
|
|
|
|
Implemented the read-only discovery foundation. The FDB database was not opened,
|
|
inspected, or modified.
|
|
|
|
## Files Changed
|
|
|
|
- `src/gti_discovery/__init__.py`: exports the read-only public interface.
|
|
- `src/gti_discovery/read_only.py`: validates one `SELECT`/`WITH` statement and
|
|
exposes `ReadOnlyConnection.execute` without connection write methods.
|
|
- `tests/test_read_only.py`: covers accepted queries, normalization, prohibited
|
|
statements, comments, multiple statements, quoted literals, and delegation.
|
|
- `pyproject.toml`: defines the package and `src`-layout pytest configuration.
|
|
- `.env.example`: documents an empty database-path placeholder without secrets.
|
|
- `CLAUDE.md`: records the strict read-only project constraints and test command.
|
|
|
|
## Tests
|
|
|
|
Command:
|
|
|
|
```text
|
|
python -m pytest tests/test_read_only.py -q
|
|
```
|
|
|
|
Result: `18 passed in 0.04s`.
|
|
|
|
`git diff --check` also completed without whitespace errors.
|
|
|
|
## Concerns
|
|
|
|
- Commit was not created because Git has no configured author identity (`Author
|
|
identity unknown`). Git configuration was not changed.
|
|
- Pytest emits an existing `pytest-asyncio` deprecation warning because its event
|
|
loop fixture scope is not configured; the focused tests still pass.
|
|
- The validator intentionally rejects all SQL comments and any prohibited keyword
|
|
outside quoted values. It does not attempt to parse the full Firebird grammar.
|
|
- The wrapper expects the injected driver connection to provide `execute(sql,
|
|
params)`; no Firebird driver is installed or required for this offline task.
|
|
|
|
## Reviewer Fixes
|
|
|
|
- Replaced the retained raw driver connection with a narrow execute-only
|
|
delegate and added coverage proving `_connection`, `commit`, and `rollback`
|
|
are unavailable on `ReadOnlyConnection`.
|
|
- Added rejection for selectable procedure calls, `GEN_ID`, generator value
|
|
expressions, `WITH LOCK`, and `FOR UPDATE`. The wrapper cannot enforce the
|
|
driver's transaction/read-only mode, so deployment must use a read-only
|
|
database account or driver-level setting.
|
|
- Replaced global `split`/`join` normalization with whitespace collapsing only
|
|
outside quoted literals; quoted contents are preserved byte-for-byte.
|
|
- Added regression tests for side-effecting SELECT constructs and literal
|
|
preservation.
|
|
|
|
## Fix Verification
|
|
|
|
Exact command:
|
|
|
|
```text
|
|
python -m pytest tests -q
|
|
```
|
|
|
|
Exact output:
|
|
|
|
```text
|
|
C:\Users\Gustavo\AppData\Local\Programs\Python\Python312\Lib\site-packages\pytest_asyncio\plugin.py:299: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
|
|
The event loop scope for asynchronous fixtures will default to the "fixture" caching scope. Future versions of pytest-asyncio will default the fixture scope to "function". Set the default fixture loop scope explicitly to avoid unexpected behavior.
|
|
|
|
warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
|
|
....................... [100%]
|
|
23 passed in 0.06s
|
|
```
|
|
|
|
## Re-review Fixes
|
|
|
|
- Replaced the instance-bound driver delegate with an opaque-token lookup in a
|
|
module-private registry. `ReadOnlyConnection` stores only the token; its
|
|
bound `execute` method has the wrapper as `__self__` and no closure.
|
|
- Added a regression test that checks `__self__`, `__closure__`, wrapper slots,
|
|
and reachable slot values cannot expose the raw driver or `commit`.
|
|
- Added conservative rejection of unknown and qualified function calls, while
|
|
allowing a small deterministic-function allowlist. This covers selectable
|
|
procedures, `GEN_ID`, UDF/external-function style calls, generator values,
|
|
locking, and update forms. True read-only enforcement still requires a
|
|
database read-only account or read-only transaction/driver configuration.
|
|
|
|
## Re-review Verification
|
|
|
|
Exact command:
|
|
|
|
```text
|
|
python -m pytest tests -q
|
|
```
|
|
|
|
Exact output:
|
|
|
|
```text
|
|
C:\Users\Gustavo\AppData\Local\Programs\Python\Python312\Lib\site-packages\pytest_asyncio\plugin.py:299: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
|
|
The event loop scope for asynchronous fixtures will default to the "fixture" caching scope. Future versions of pytest-asyncio will default the fixture scope to "function". Set the default fixture loop scope explicitly to avoid unexpected behavior.
|
|
|
|
warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
|
|
.......................... [100%]
|
|
26 passed in 0.07s
|
|
```
|
|
|
|
## Third Fix Round
|
|
|
|
- Removed the module-level bound-method registry. `ReadOnlyConnection` now
|
|
accepts only a narrow adapter-supplied execute callable and exposes no driver
|
|
or connection attributes. Python process-level introspection is explicitly
|
|
outside this guarantee.
|
|
- Rejected all function-like SQL calls except a small deterministic-function
|
|
allowlist, including qualified names with arbitrary whitespace around dots.
|
|
Selectable procedures in `FROM`, generator calls, `GEN_ID`, `EXECUTE`, UDF,
|
|
and external-function-style calls are rejected conservatively.
|
|
- Documented that actual read-only enforcement requires a Firebird read-only
|
|
account or read-only transaction/driver configuration.
|
|
- Added regression coverage for spaced qualified calls and the practical public
|
|
API boundary.
|
|
|
|
## Third Fix Verification
|
|
|
|
Exact command:
|
|
|
|
```text
|
|
python -m pytest tests -q
|
|
```
|
|
|
|
Exact output:
|
|
|
|
```text
|
|
C:\Users\Gustavo\AppData\Local\Programs\Python\Python312\Lib\site-packages\pytest_asyncio\plugin.py:299: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
|
|
The event loop scope for asynchronous fixtures will default to the "fixture" caching scope. Future versions of pytest-asyncio will default the fixture scope to "function". Set the default fixture loop scope explicitly to avoid unexpected behavior.
|
|
|
|
warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
|
|
................................... [100%]
|
|
35 passed in 0.09s
|
|
```
|