-
Notifications
You must be signed in to change notification settings - Fork 0
codex(v1 base): packaged Python reference (X25519/HKDF/ChaCha20-Poly1305), enforceable crypto policy, canonical receipts, Makefile, CODEOWNERS, and supply-chain guardians #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @VaultSovereign, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This PR introduces the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is a fantastic pull request that introduces a well-structured and modern Python package for cryptographic operations, complete with policy enforcement, CI/CD integration, and clear documentation. The design choices are well-reasoned and documented in the pull request description.
I've identified a few areas for improvement, primarily concerning security hardening, robustness, and adherence to Python best practices. My comments are detailed below.
key = _hkdf_sha256(shared, salt, b"codex/seal/v1", 32) | ||
nonce = base64.b64decode(env.nonce) | ||
aad_in = aad if aad is not None else b"" | ||
if hashlib.sha256(aad_in).hexdigest() != env.aad_hash: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The direct string comparison of hashes using !=
is vulnerable to timing attacks. An attacker could potentially discern information about the hash by measuring the time it takes for the comparison to fail. It's recommended to use a constant-time comparison function like hmac.compare_digest
for this purpose.
Make sure to add import hmac
at the top of the file.
if hashlib.sha256(aad_in).hexdigest() != env.aad_hash: | |
if not hmac.compare_digest(hashlib.sha256(aad_in).hexdigest(), env.aad_hash): |
import hashlib | ||
import json | ||
import os | ||
from dataclasses import dataclass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To enable robustly converting the Envelope
dataclass to a dictionary, please import asdict
from the dataclasses
module. It will be used in the envelope_to_bytes
function to provide a more idiomatic and safer serialization method than accessing __dict__
.
from dataclasses import dataclass | |
from dataclasses import asdict, dataclass |
ct_b64 = base64.b64encode(salt + ct).decode("ascii") # prefix salt for key derivation | ||
aad_hash = hashlib.sha256(aad_in).hexdigest() | ||
|
||
return Envelope(VERSION, dict(ALG), eph_pub_b64, nonce_b64, ct_b64, aad_hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Envelope
dataclass is defined with frozen=True
, which makes its instances immutable. Therefore, creating a shallow copy of the ALG
dictionary with dict(ALG)
is unnecessary. You can pass ALG
directly. This avoids a small performance overhead and simplifies the code.
return Envelope(VERSION, dict(ALG), eph_pub_b64, nonce_b64, ct_b64, aad_hash) | |
return Envelope(VERSION, ALG, eph_pub_b64, nonce_b64, ct_b64, aad_hash) |
|
||
|
||
def envelope_to_bytes(env: Envelope) -> bytes: | ||
return _jcs(env.__dict__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.makedirs(outdir, exist_ok=True) | ||
receipt = Receipt(kind=kind, ts_ms=_now_ms(), id=str(uuid.uuid4()), payload=payload) | ||
data = _jcs({"receipt": asdict(receipt), "sha256": receipt.digest(), "codex_version": version}) | ||
fname = f"{outdir}/{receipt.ts_ms}_{kind}.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constructing file paths with f-strings and /
is not portable across different operating systems (e.g., Windows). It's a best practice to use os.path.join()
to ensure your code runs correctly everywhere.
fname = f"{outdir}/{receipt.ts_ms}_{kind}.json" | |
fname = os.path.join(outdir, f"{receipt.ts_ms}_{kind}.json") |
if str(SRC) not in sys.path: | ||
sys.path.insert(0, str(SRC)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modifying sys.path
directly is generally considered an anti-pattern. The Makefile
's install
target already uses pip install -e .[dev]
, which makes the codex
package available to pytest
in a standard way.
This sys.path
manipulation is therefore redundant and could cause confusion by making it unclear whether tests are running against the installed package or the raw source files. It's best to remove it and rely on the editable installation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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
[project] | ||
name = "encryption-standards-codex" | ||
version = "0.0.1" | ||
description = "VaultMesh Encryption Standards Codex: reference primitives, policy, and receipts." | ||
requires-python = ">=3.10" | ||
authors = [{name="VaultSovereign"}] | ||
dependencies = [ | ||
"cryptography>=42.0.0", | ||
] | ||
|
||
[build-system] | ||
requires = ["setuptools>=69", "wheel"] | ||
build-backend = "setuptools.build_meta" | ||
|
||
[project.optional-dependencies] | ||
dev = [ | ||
"pytest>=7.4", | ||
"mypy>=1.5.0", | ||
"ruff>=0.5.0", | ||
"bandit>=1.7.5", | ||
] | ||
|
||
[tool.ruff] | ||
line-length = 100 | ||
target-version = "py311" | ||
|
||
[tool.mypy] | ||
python_version = "3.11" | ||
warn_unused_ignores = true | ||
warn_redundant_casts = true | ||
strict = true | ||
|
||
[tool.bandit] | ||
skips = ["B101"] | ||
|
||
[tool.pytest.ini_options] | ||
addopts = "-q" | ||
testpaths = ["tests"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Package build excludes codex modules
The new pyproject.toml
declares a src/ layout but never tells setuptools to look inside src
. Without a [tool.setuptools.packages.find]
or similar packages = {"": "src"}
entry, pip install .
produces a wheel with no Python modules and an installed environment cannot import codex
. Because packaging the reference implementation is a central deliverable of this commit, the library is effectively unusable when installed from a distribution.
Useful? React with 👍 / 👎.
def write_receipt(kind: str, payload: dict, outdir: str = "receipts") -> str: | ||
os.makedirs(outdir, exist_ok=True) | ||
receipt = Receipt(kind=kind, ts_ms=_now_ms(), id=str(uuid.uuid4()), payload=payload) | ||
data = _jcs({"receipt": asdict(receipt), "sha256": receipt.digest(), "codex_version": version}) | ||
fname = f"{outdir}/{receipt.ts_ms}_{kind}.json" | ||
with open(fname, "w", encoding="utf-8") as handle: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Receipts overwrite when generated in same millisecond
write_receipt
names output files as {ts_ms}_{kind}.json
. Because the filename omits the UUID and only uses millisecond precision, two receipts of the same kind created within the same millisecond produce the same path and the later call silently overwrites the earlier file. This risks losing audit records under bursts of activity. Include the receipt ID (or open with exclusive mode) to ensure filenames are unique.
Useful? React with 👍 / 👎.
What this delivers (high-signal TL;DR)
Packaged Python reference under
src/codex/…
with:codex.crypto
)codex.keys
)codex.receipts
)tests/test_crypto.py
)Policy guardrails: OPA Rego crypto policy (
policy/crypto_policy.rego
) +make policy
gate.Supply-chain guardians wired via CI: OpenSSF Scorecard, SBOM (CycloneDX), Dependency Review, Dependabot.
Developer ergonomics:
Makefile
withvenv
,install
,lint
,typecheck
,test
,policy
,receipts-demo
.Stewardship clarity: CODEOWNERS coverage for critical paths.
Docs refresh: modern README with algorithms, quickstart, Security & Receipts sections.
Rationale (why this shape)
import codex.crypto
) instead of copy-pasting scripts.Code layout & responsibilities
Reference crypto (envelope) — design notes
X25519
(ephemeral sender → recipient static)info="codex/seal/v1"
os.urandom(12)
ciphertext = base64(salt || aead(nonce, plaintext, aad))
(salt is prefixed)Integrity & Replay: AEAD enforces integrity; callers are expected to bind AAD to a context (e.g., realm/document id/version). Receipts include
sha256
over the canonical envelope for audit.Receipts — canonical, hash-bound events
codex.receipts.write_receipt(kind, payload)
writesreceipts/<ts>_<kind>.json
with:receipt
:{kind, ts_ms, id, payload, domain_version}
sha256
:SHA256(JCS(receipt))
codex_version
: module versionThese files can be Merkle-compacted upstream (VaultMesh sovereign ledger).
Demo:
Policy enforcement (OPA)
Rule: allow only
{ aead=chacha20poly1305, kdf=hkdf-sha256, dh=x25519, sig=ed25519 }
…and minimum sizes:
x25519>=32
,ed25519>=32
,nonce=12
.Run gate locally:
Developer workflow (Make targets)
CI & guardians (what runs and when)
CI (lint/type/test/bandit):
.github/workflows/ci.yml
src/**
,tests/**
,pyproject.toml
,policy/**
,Makefile
Dependency Review: blocks PRs that introduce known-bad upgrades.
SBOM: Syft →
sbom.cdx.json
artifact; path-scoped + concurrency guard.OpenSSF Scorecard: SARIF uploaded to Security tab.
Dependabot: weekly for
github-actions
and pip (scoped to package dir).Documentation updates (README)
make install
→make test
).Security doctrine (operational)
seal
) to guarantee PFS against key compromise of recipients.realm|doc|version
).-q
) with known false-positive skips configurable viapyproject.toml
.Interop & forward compatibility
version=1
,alg
map included for explicitness.Testing
PYTHONPATH=src pytest -q
✅ruff
,mypy
,pytest
,bandit
.pip install -e ".[dev]"
fails behind proxies with no PyPI egress. Resolve via local wheel mirror or internal index (see Offline note above).Backward/related files touched (highlights)
LICENSE
header normalized to VaultSovereign.CODEOWNERS
spans.github/
,policy/
,src/
,tests/
,Makefile
,pyproject.toml
.Release ritual (suggested)
v0.1.0-solid
(or-sealed
to mirror sovereign)sbom.cdx.json
(from SBOM workflow),codex-tools.tar.gz
(if you keep the bundling step)Future work (fast follow)
codex
entry points:codex seal/open/sign/verify/rotate
with receipts emission.Operator quick commands
Closing seal
Solve et Coagula. The codex now stands as a living standard: secure by default, policy-enforced, receipt-emitting, and supply-chain guarded. Each invocation writes memory; each memory admits a proof; each proof strengthens the civilization.
https://chatgpt.com/codex/tasks/task_e_68e641489cf4832db6e16d8be4745ebf