fix: OAuth2 token encoding and /auth/info route - #49
Conversation
Coding-Dev-Tools
commented
Aug 9, 2026
Coding-Dev-Tools
commented
- URL-encode 1Password Connect filter keys for special characters\n- Encode OAuth2 introspection tokens\n- Add /auth/info route and tests\n- Fix dead code in serve.py
…l characters OnePasswordStore.get() and delete() injected the key directly into the filter query parameter without URL encoding. Keys containing &, =, #, spaces, or quotes produced malformed URLs and failed to match. Fix: apply urllib.parse.quote(key, safe='') before embedding in the filter string, matching the pattern used by serve.py OAuth2 tokens. Added 2 regression tests covering get/delete with special-character keys.
🤖 Automated Code Review✅ Ruff Lint — No issues
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 343b88f353
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Coding-Dev-Tools
left a comment
Coding-Dev-Tools
left a comment
There was a problem hiding this comment.
Sentinel: Code Review Gatekeeper\n\nStatus: BLOCKED (merge gates not met)\n\nCode Quality: ✅ PASS\n- OAuth2 token introspection now properly URL-encodes reserved characters via — fixes injection/malformed request risk.\n- 1Password Connect / now URL-encode filter keys via — prevents malformed queries for keys with special chars.\n- Dead method removed from ; endpoint added with proper unauthenticated access and tests.\n- Ruff formatting applied to test files.\n- All CI checks passing (3.11, 3.12, 3.13 + automated code review).\n\nSecurity: ✅ No secrets, credentials, or unsafe patterns detected. Encoding fixes are security-positive.\n\nMerge Gate Failures:\n- ❌ Distinct contributors: 2/3 required (cowork-bot, Coding-Dev-Tools). Need at least 1 more distinct agent author.\n- ❌ Reviewer approvals: 0/3 required. Codex left suggestions but no formal approval.\n- ❌ PR age: ~9 hours old — passes 6h minimum.\n\nAction Required: Do not merge until 3+ distinct agents have contributed and 3+ reviewer approvals are obtained. Code changes themselves are approved.
Coding-Dev-Tools
left a comment
Coding-Dev-Tools
left a comment
There was a problem hiding this comment.
Sentinel: Code Review Gatekeeper
Status: BLOCKED (merge gates not met)
Code Quality: ✅ PASS
- OAuth2 token introspection now properly URL-encodes reserved characters via
urlencode()— fixes injection/malformed request risk. - 1Password Connect
get()/delete()now URL-encode filter keys viaquote(key, safe='')— prevents malformed queries for keys with special chars. - Dead
_check_authmethod removed fromserve.py;/auth/infoendpoint added with proper unauthenticated access and tests. - Ruff formatting applied to test files.
- All CI checks passing (3.11, 3.12, 3.13 + automated code review).
Security: ✅ No secrets, credentials, or unsafe patterns detected. Encoding fixes are security-positive.
Merge Gate Failures:
- ❌ Distinct contributors: 2/3 required (cowork-bot, Coding-Dev-Tools). Need at least 1 more distinct agent author.
- ❌ Reviewer approvals: 0/3 required. Codex left suggestions but no formal approval.
- ✅ PR age: ~9 hours old — passes 6h minimum.
Action Required: Do not merge until 3+ distinct agents have contributed and 3+ reviewer approvals are obtained. Code changes themselves are approved.
…ad of discarding all
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0831ab7f50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Closes coverage gaps in BearerAuth, ApiKeyAuth, OAuth2Auth (userinfo, introspect, cache, scope/audience validation, error paths), MultiAuth fallback logic, and build_auth_from_env factory. 527 tests pass, ruff clean.
… automated code review
…nd; batch Doppler get_many
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fd3be4f5b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if type(exc).__name__ == "InvalidPath": | ||
| return True | ||
| message = str(exc).lower() | ||
| return "404" in message or "not found" in message or "path" in message and "missing" in message |
There was a problem hiding this comment.
When a real Vault failure contains 404 for another reason—such as a connection error to vault.internal:4040 or a permission error whose requested path contains 404—this predicate classifies it as a missing secret. The new get, delete, and list_keys handlers then silently return None, False, or [] instead of surfacing the operational failure they are intended to preserve; inspect the exception type or structured HTTP status rather than searching the entire message for this substring.
Useful? React with 👍 / 👎.
…te_env_var rotate_all previously rewrote the .env file once per key via N sequential read-modify-write cycles: slow, and a crash mid-loop left the file partially rotated. New rotate_env_file() plans all replacements, applies them in one pass over the lines, and writes atomically (temp file + fsync + os.replace) so a crash can never leave a truncated/half-rotated .env. CLI rotate-all now uses it. 6 regression tests added.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d667c835f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """ | ||
| tmp = path.with_name(f".{path.name}.rotate-tmp-{os.getpid()}") | ||
| try: | ||
| with open(tmp, "w") as f: |
There was a problem hiding this comment.
When the source .env is secured with mode 0600 and the process has a typical 0022 umask, opening this new temporary file creates it as 0644; os.replace() then installs those permissions on the rotated .env. A successful rotate-all therefore makes every newly generated secret readable by other local users, so copy the original file mode to the temporary file before replacing it.
Useful? React with 👍 / 👎.
| if m and m.group(1) in plan and m.group(1) not in seen: | ||
| key = m.group(1) | ||
| seen.add(key) |
There was a problem hiding this comment.
For a file containing the same key more than once, such as TOKEN=first followed by TOKEN=effective, dotenv_values() places one TOKEN in the plan but this seen condition rewrites only the first occurrence. The later occurrence remains unchanged and continues to be the effective value when the file is loaded, even though the command reports and audits a successful rotation.
Useful? React with 👍 / 👎.
| f.write(content) | ||
| f.flush() | ||
| os.fsync(f.fileno()) | ||
| os.replace(tmp, path) |
There was a problem hiding this comment.
When the configured environment file is a symlink, the preceding read follows the link but os.replace(tmp, path) replaces the symlink itself with a regular file. This leaves the original shared/generated target containing the old secrets and silently detaches this environment from future target updates; resolve the target before creating and replacing the temporary file, or otherwise preserve the link.
Useful? React with 👍 / 👎.
- _get_commit_meta: NUL (%x00) field separators so author names/subjects
containing '|' no longer silently drop every change of a commit; run the
lookup inside the target repo via cwd instead of the process CWD
- pass POSIX ('/') paths to git log/show so nested .env files resolve on
Windows (backslash pathspecs matched nothing -> empty history)
- add tests/test_history.py: meta parsing with '|' authors, mask behavior,
end-to-end history detection in a real temp repo incl. nested path
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
