bridge: make v3 pass the v2 REST suite - #61
Open
aberoham wants to merge 48 commits into
Open
Conversation
This was referenced Aug 25, 2026
aberoham
marked this pull request as ready for review
August 25, 2026 20:47
Contributor
|
I cancelled the tests, after not passing for an hour, they weren't going to. |
msimerson
pushed a commit
to NicTool/validate
that referenced
this pull request
Aug 27, 2026
Companion to NicTool/api#61 and NicTool/NicTool#365.
Contributor
|
needs updating against validate 1.0.0 |
There was a problem hiding this comment.
Pull request overview
Extends the v3 API to support v2 GUI traffic through the REST bridge for NicTool/NicTool#365.
Changes:
- Adds centralized authorization, delegation, session revalidation, and audit logging.
- Aligns MySQL and file stores with pagination, scoped reads, moves, and canonical zone uniqueness.
- Expands backend and integration tests and upgrades
@nictool/validate.
Reviewed changes
Copilot reviewed 79 out of 79 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
test/run.sh |
Routes coverage through backend runners. |
test/backends/toml.sh |
Runs TOML tests serially. |
test/backends/mysql.sh |
Runs MySQL tests serially. |
test/backends/json.sh |
Runs JSON tests serially. |
routes/zone.test.js |
Tests uniqueness, moves, and audit logs. |
routes/zone.js |
Adds authorization, pagination, auditing, and conflicts. |
routes/zone_record.test.js |
Tests delegation scope, sorting, and auditing. |
routes/zone_record.js |
Adds authorization, scope, and auditing. |
routes/user.test.js |
Tests sessions, searches, moves, and permissions. |
routes/user.js |
Adds authorization, pagination, moves, and permission controls. |
routes/test/permissions.js |
Adds route permission fixtures. |
routes/session.test.js |
Tests revocation and qualified usernames. |
routes/session.js |
Returns effective permissions and refreshes activity. |
routes/permission.test.js |
Tests self-permission restrictions. |
routes/permission.js |
Adds authorization and permission updates. |
routes/nameserver.test.js |
Tests nameserver moves. |
routes/nameserver.js |
Adds authorization and scoped listings. |
routes/log.js |
Introduces audit-log endpoints. |
routes/lifecycle.test.js |
Tests shutdown disconnection. |
routes/index.js |
Registers authz, delegation, logs, and shutdown hook. |
routes/group.test.js |
Tests authorized group operations. |
routes/group.js |
Adds authorization and permission controls. |
routes/delegation.js |
Introduces delegation routes and caps. |
package.json |
Upgrades validation dependency. |
lib/zone/test/mysql.js |
Tests canonical lock names. |
lib/zone/store/mysql.js |
Adds locking, scoping, and nameserver access. |
lib/zone/store/file.js |
Adds uniqueness locking and scoped access. |
lib/zone/store/base.js |
Defines zone-name canonicalization and conflicts. |
lib/zone_record/test/index.js |
Tests record-type replacement. |
lib/zone_record/store/mysql.js |
Adds scoping, sorting, and type cleanup. |
lib/zone_record/store/file.js |
Adds scoped filtering and expanded sorting. |
lib/user/test/index.js |
Tests creation, listing, and authentication. |
lib/user/store/mysql.js |
Adds search, pagination, moves, and qualified login. |
lib/user/store/file.js |
Aligns file-backed user behavior. |
lib/user/qualified.test.js |
Tests qualified username parsing. |
lib/user/qualified.js |
Adds qualified username parsing. |
lib/store-stubs.test.js |
Verifies unsupported backend stubs. |
lib/store-access.test.js |
Enforces store-only MySQL access. |
lib/session/test/index.js |
Tests distinct login sessions. |
lib/session/store/mysql.js |
Preserves distinct sessions and activity timestamps. |
lib/session/store/file.js |
Serializes session creation. |
lib/permission/test/index.js |
Tests legacy rows and permission reuse. |
lib/permission/store/mysql.js |
Handles legacy rows and partial updates. |
lib/permission/store/mongodb.js |
Adds an explicit unsupported stub. |
lib/permission/store/file.js |
Expands file-backed permission behavior. |
lib/permission/store/elasticsearch.js |
Adds an explicit unsupported stub. |
lib/permission/index.js |
Dispatches additional backends. |
lib/page.test.js |
Tests pagination limits. |
lib/page.js |
Adds configurable page limits. |
lib/mysql.js |
Adds dedicated transactional connections. |
lib/group/test/index.js |
Tests ancestry rebuilding after moves. |
lib/group/store/mysql.js |
Rebuilds subgroup closure transactionally. |
lib/group/store/file.js |
Delegates permission creation to its store. |
lib/file-stores.test.js |
Tests file-store subsystem parity. |
lib/delegation/store/mysql.js |
Implements MySQL delegation persistence. |
lib/delegation/store/mongodb.js |
Adds an explicit unsupported stub. |
lib/delegation/store/file.js |
Implements file-backed delegations. |
lib/delegation/store/elasticsearch.js |
Adds an explicit unsupported stub. |
lib/delegation/store/base.js |
Defines the delegation contract. |
lib/delegation/index.js |
Selects the delegation backend. |
lib/config.js |
Adds a list-limit environment override. |
lib/authz/store/mysql.js |
Implements MySQL authorization queries. |
lib/authz/store/mongodb.js |
Adds an explicit unsupported stub. |
lib/authz/store/file.js |
Implements file-backed authorization queries. |
lib/authz/store/elasticsearch.js |
Adds an explicit unsupported stub. |
lib/authz/store/base.js |
Implements authorization policy. |
lib/authz/index.js |
Selects the authorization backend. |
lib/authz-plugin.js |
Enforces authorization and session validity. |
lib/audit/store/mysql.js |
Implements MySQL audit persistence and queries. |
lib/audit/store/mongodb.js |
Adds an explicit unsupported stub. |
lib/audit/store/file.js |
Implements file-backed audit storage. |
lib/audit/store/elasticsearch.js |
Adds an explicit unsupported stub. |
lib/audit/store/base.js |
Defines audit logging behavior. |
lib/audit/index.js |
Selects the audit backend. |
lib/audit.test.js |
Tests MySQL audit behavior. |
docker/Dockerfile |
Normalizes package copy syntax. |
CHANGELOG.md |
Documents unreleased bridge changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
aberoham
force-pushed
the
gui-rest-transport
branch
2 times, most recently
from
August 29, 2026 18:02
c232f75 to
583225e
Compare
v3 didn't have permission enforcement yet -- this adds it. Hapi onPreHandler plugin reads route metadata and runs checks before handlers execute, rather than scattering permission calls inside each one. authz.js is the engine (checkPermission, group tree walks, delegation lookups). authz-plugin.js wires it into Hapi's request lifecycle. All routes annotated with what they need. Delegation routes now cap submitted permissions by the caller's own permissions at write time, which matches how v2 does it. Unit tests for the Authz class and integration tests via server.inject() included. v2 xt permission tests (14_permissions, 20_permission) should still pass -- 4892/4892 last run.
New subsystems reached for Mysql directly; route them through lib/<subsystem>/store/ with base contracts, mysql implementations, and json file stores so a file-store deployment gets the whole api. Group moves run on a dedicated transaction connection instead of the shared one, whose reads could not see their own uncommitted writes. List endpoints gain an operator-tunable page ceiling (http.list_limit_max, default 1000). lib/store-access.test.js fails on any mysql import outside store modules.
PUT /user/{self} applied any permission fields in the payload through
Permission.put, so a self_write holder could grant themselves rights
that /permission denies for self-targeted writes. Profile fields still
apply; permission fields on a self edit are dropped.
validate#29 shipped as 1.0.0, so the temporary install of its PR head in CI, coverage, and the docker build can go. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
A record-only delegation reads the parent zone, so /log/zone_record returned the history of every record in it. Apply the same read scope the record listing uses and let both audit stores take the id list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
The delegation store treats an omitted type as ZONE; the plugin refused the same request as an unknown type before it got there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
The active-only read after a PUT that sets deleted found nothing and the handler threw on it. Read the deleted row back, as the zone route does, and log the deletion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
nt_delegate has no unique key, so identical concurrent POSTs could both pass the check. Run it with the insert in a transaction under a lock named for the delegation; the audit row is written after commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
Inline permission rows were returned whatever their deleted flag, so a deleted explicit permission kept authorizing and never fell back to the group. Return a row only in the state asked for, replace a deleted row on create as the mysql store does, and let a lookup by id reach group rows after user and standalone ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
The v2 GUI posts a zone's nameserver selection on create and edit, and
v3 only exposed a read-only /zone/{id}/ns. Zone create and put now take
`nameservers` (nameserver ids) and store them through the zone store,
inside the zone's transaction on mysql and under the write lock on the
file store; a single-zone read carries the assignment back. A caller
may only assign nameservers it can use: owned by a group in its tree,
or granted through usable_ns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
The zone nameservers routes need the schema from validate#31, merged as 0a09c61 but not released. Drop this once the release lands and the dependency bumps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
The audit and delegation stores loaded, appended, and saved without a lock, so overlapping writers could hand out the same id and the later save dropped the earlier row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
Creating a zone without an id stored and returned id 0 where mysql hands back its auto-increment id. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
The log scope fix had no test showing a record-only delegate sees only its own record's history. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
routes/authz.test.js only races creates on mysql. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
A database created by 2.x defaults nt_user.is_admin to 0, so the stripped field reads back 0 there and null on a v3 install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RxcUNWRPrY7gKukije8hFC
aberoham
force-pushed
the
gui-rest-transport
branch
from
August 29, 2026 18:54
583225e to
b516a4f
Compare
NicTool#66's discover_tests put routes/authz.test.js in the json and toml suites, where the mysql-only skip failed the metarepo's no-skips gate. The file store serializes writes, so the race holds there too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GiMQBtdjyPFutmvYZpZMy7
This was referenced Aug 29, 2026
aberoham
added a commit
to NicTool/metarepo
that referenced
this pull request
Aug 29, 2026
Agents kept missing rules a diff can check, most recently a comment restating the banner under it (NicTool/NicTool#369). Those rules now run in git, whoever is committing: `hooks/check.py` refuses comment runs over two lines, a comment repeating five words of code within 40 lines, the `AGENTS.md` word list in comments and commit messages, and subjects that are capitalised or over 72 chars. `./nt.py sync` sets `core.hooksPath` in every clone; `./nt.py lint` runs the same checks over claimed branches, which matters because rebases and cherry-picks skip commit hooks. `git commit --no-verify` stays the deliberate exception. Meaning is still a review job. Cost, stated up front: lint already flags open branches — one hit on NicTool `gui-rest-transport` (`dist/setup/setup-test-env.pl:93`) and eight over-length comments on NicTool/api#61. They need tidying before either branch commits cleanly with the hooks on. Standard library only; 24 fixture tests plus an end-to-end run in a temp repo.
This was referenced Aug 29, 2026
The mysql run never loads the file stores, so their branches count as uncovered and the project number lands under main's. The json store measures 93.17% over the files it exercises. Each store keeps its own lcov report and codecov receives both for the commit. The mysql report stays at coverage/lcov.info, where coveralls reads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bridge: make v3 pass the v2 REST suite
Everything the v2 GUI needs from the v3 API to serve real traffic through the
REST bridge (NicTool/NicTool#365).
authz enforcement. Ports the v2 permission model: group-tree reads, per-resource caps,
delegations with pseudo-delegation for zone records (
zone_perm_add_records/zone_perm_delete_records), self-write limits, and session revalidation against live user/group rows with a 4h idle cutoff. Enforcement lives in one hapi plugin; policy in
lib/authz/store/base.js, SQL inlib/authz/store/mysql.js.store layer. Audit, authz, delegation, and permission data access move behind
lib/<entity>/store/— mysql complete, json/toml file backends aligned with mysql behavior,mongodb/elasticsearch stubs that throw loudly. No
mysql2imports outside stores;lib/store-access.test.jsguards that.Also here: searchable/paginated user listings, object moves between groups (with
permission-row rewrites), audit logging on zone/record mutations, canonical zone-name
conflict detection, scoped reads so delegated collections only show delegated rows.
Two semantic decisions worth review:
permissionRecordresolves a user-targeted perm row's group viaCOALESCE(u.nt_group_id, NULLIF(p.nt_group_id, 0))— the user's current group wins overwhat the row says. Stale v2-era rows stop granting/denying based on dead group placement,
and moves rewrite them going forward.
(case/trailing-dot folded). There is deliberately no unique index, so writers outside this
codepath (v2 against the shared DB) can still create canonically-duplicate zones. Say the
word if you'd rather have the index.
Copilot review fixes included: self_write can no longer smuggle permission fields through
PUT /user/{self}; moving a record out of a zone requires delete permission at the source, notjust create at the destination; session activity refreshes on ordinary requests so the idle
cutoff measures idleness.
Second-review fixes: the JWT max age now matches the 8h token ttl (the 4h cap cut active
users off regardless of activity); the audit and delegation file stores serialize their writes;
the zone file store allocates ids;
/log/zone_recordhas a record-only delegation test.Depends on: validate#31 (merged as 0a09c61, not yet released). CI installs that commit over
the published 1.0.0; the dependency bumps to the next validate release when it ships, and that
commit goes away. Pairs with NicTool/NicTool#365 (bridge + xt) and #363 (password hash interop).
Supersedes #60.
Tests: full suite green across api lib/routes and the dns libs; file-store suites pass under
NICTOOL_DATA_STORE=json; xt suites run through the bridge in NicTool#365.