-
-
Notifications
You must be signed in to change notification settings - Fork 34
Sync master to dev (fast-follow): audit fixes #1842/#1846/#1847 #1849
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
Changes from all commits
edafd5c
c892227
02c767f
8e3b2c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -372,7 +372,9 @@ Environment=HAILO_OLLAMA_PORT=$HAILO_OLLAMA_PORT | |
| # hailo-ollama can leave a bare orphan process listening on the port if it | ||
| # crashes during model load (the adopt-an-orphan lesson from PR #1755 for | ||
| # rkllama). Reap any such process before (re)start so the bind cannot fail. | ||
| ExecStartPre=-/usr/bin/pkill -9 -f 'hailo-ollama' | ||
| # Match the exact server invocation, not any command line merely containing | ||
| # "hailo-ollama" (which would kill an editor or tail open on these files). | ||
| ExecStartPre=-/usr/bin/pkill -9 -f "$HAILO_OLLAMA_BIN serve --port $HAILO_OLLAMA_PORT" | ||
| ExecStart=$exec_start | ||
| Restart=always | ||
| RestartSec=5 | ||
|
|
@@ -410,7 +412,13 @@ already_installed() { | |
| # * systemd unit enabled | ||
| # * HTTP API responding with a "models" body | ||
| [[ -d "$HAILO_OLLAMA_DIR/.git" ]] || return 1 | ||
| [[ "$(run_as_user git -C "$HAILO_OLLAMA_DIR" rev-parse HEAD 2>/dev/null || true)" == "$HAILO_OLLAMA_REF" ]] || return 1 | ||
| # Compare resolved commits, not HEAD-vs-ref-name: HAILO_OLLAMA_REF is usually | ||
| # a branch/tag ("main"), so comparing it to rev-parse HEAD (a SHA) never | ||
| # matched and every re-run needlessly re-fetched and re-installed. | ||
| local _head _want | ||
| _head="$(run_as_user git -C "$HAILO_OLLAMA_DIR" rev-parse HEAD 2>/dev/null || true)" | ||
| _want="$(run_as_user git -C "$HAILO_OLLAMA_DIR" rev-parse "${HAILO_OLLAMA_REF}^{commit}" 2>/dev/null || true)" | ||
| [[ -n "$_head" && "$_head" == "$_want" ]] || return 1 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION:
Reply with |
||
| systemctl is-enabled hailo-ollama.service >/dev/null 2>&1 || return 1 | ||
| local tags | ||
| tags="$(curl -fs "http://localhost:$HAILO_OLLAMA_PORT/api/tags" 2>/dev/null || true)" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,7 +208,6 @@ async def verify_chain(store: hub_store.HubStore, author: str) -> tuple[bool, Op | |
| which is exactly why the chain index must persist past deletion. | ||
| """ | ||
| entries = await store.list_chain_entries(author) | ||
| pub = identity.public_identity()["signing_pubkey"] | ||
| prev_hash: Optional[str] = None | ||
| for entry in entries: | ||
| if entry.get("prev_hash") != prev_hash: | ||
|
|
@@ -218,8 +217,16 @@ async def verify_chain(store: hub_store.HubStore, author: str) -> tuple[bool, Op | |
| f"does not link to previous entry {prev_hash!r} (chain broken)", | ||
| ) | ||
| obj = await store.get_object(entry["hash"]) | ||
| if obj is not None and not hub_store.verify_object(obj, pub): | ||
| return False, f"seq {entry['seq']}: signature invalid (tampered)" | ||
| if obj is not None: | ||
| # Verify each entry against the public key resolved from that entry's | ||
| # declared author (its signing fingerprint), not one assumed key. A | ||
| # directory/network mishap that pairs an entry with the wrong author | ||
| # fails the author-to-key binding in verify_object. | ||
| resolved = await store.get_author(entry["author"]) | ||
| if resolved is None or not resolved.get("signing_pubkey"): | ||
| return False, f"seq {entry['seq']}: unknown author {entry['author']!r}" | ||
| if not hub_store.verify_object(obj, resolved["signing_pubkey"]): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Previously the chain was verified with the single node identity ( Reply with |
||
| return False, f"seq {entry['seq']}: signature invalid (tampered)" | ||
| prev_hash = entry["hash"] | ||
| return True, None | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -91,10 +91,21 @@ def verify_object(obj: dict, signing_pubkey_hex: str) -> bool: | |||||||||||||||||||||||||
| Returns False (never raises) on a missing or malformed signature so callers | ||||||||||||||||||||||||||
| verifying arbitrary peer objects degrade cleanly. Tampering with any signed | ||||||||||||||||||||||||||
| field changes the canonical bytes and fails the check. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| The declared ``author`` MUST be the fingerprint (SHA-256) of the public key | ||||||||||||||||||||||||||
| that signed the object, so a valid signature from key A cannot be presented | ||||||||||||||||||||||||||
| as authored by peer B. This closes the peer-impersonation gap that opens once | ||||||||||||||||||||||||||
| peer ingest lands (today the routes force the local fingerprint, so it is not | ||||||||||||||||||||||||||
| yet exploitable). The check is rejected unless ``author`` equals that | ||||||||||||||||||||||||||
| fingerprint. | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| sig = obj.get("sig") | ||||||||||||||||||||||||||
| if not isinstance(sig, str) or not sig: | ||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||
| author = obj.get("author") | ||||||||||||||||||||||||||
| expected_author = identity.fingerprint(signing_pubkey_hex) | ||||||||||||||||||||||||||
| if not isinstance(author, str) or author != expected_author: | ||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||
| return identity.verify_signature(signing_pubkey_hex, canonical_bytes(obj), sig) | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION:
Suggested change
Reply with |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| _MIN_WIDTH = 800 | ||
| _MIN_HEIGHT = 600 | ||
| _PADDING = 40 | ||
| _MAX_RENDER_DIM = 8192 | ||
|
|
||
| _KIND_FILL = { | ||
| "note": (255, 240, 140, 255), | ||
|
|
@@ -78,6 +79,8 @@ def render_snapshot_png(*, elements: list[dict], output_path: Path) -> Path: | |
| min_x, min_y, max_x, max_y = _bounds(elements) | ||
| width = max(_MIN_WIDTH, int(max_x - min_x) + 2 * _PADDING) | ||
| height = max(_MIN_HEIGHT, int(max_y - min_y) + 2 * _PADDING) | ||
| width = min(width, _MAX_RENDER_DIM) | ||
| height = min(height, _MAX_RENDER_DIM) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: Clamping render dimensions to
Reply with |
||
| img = Image.new("RGB", (width, height), color=(255, 255, 255)) | ||
| draw = ImageDraw.Draw(img, "RGBA") | ||
| try: | ||
|
|
||
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.
SUGGESTION:
_wantresolves${HAILO_OLLAMA_REF}^{commit}. IfHAILO_OLLAMA_REFis a branch name that the local clone has never fetched (the dir exists from a prior partial clone),rev-parsereturns empty and_wantis empty, so[[ -n "$_head" && "$_head" == "$_want" ]]is false and the function returns non-zero → triggers a needless re-fetch/re-install even when HEAD is correct. The default upstream being unreachable (noted in the commit) makes a freshgit fetchlikely fail too, so this branch may always re-clone. Considergit fetchbefore resolving, or tolerating an unresolvable_wantwhen_headis already a valid commit. Low impact since reinstall is idempotent, but worth noting.Reply with
@kilocode-bot fix itto have Kilo Code address this issue.