fix(money-machine): harden desktop security#61
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR consolidates a major application update combining domain migration, comprehensive security hardening, API contract consolidation, and frontend redesign. URLs shift from ChangesUnified Product Update
🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)✅ Unit Test PR creation complete.
Comment |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| entry_price: Optional[float] = None | ||
| stop_loss: Optional[float] = None | ||
| take_profit: Optional[float] = None | ||
| amount: Optional[float] = None |
There was a problem hiding this comment.
🔴 Adding amount field to TradingSignal breaks SignalContract.from_trading_signal, silently disabling all pipeline trades
The PR adds amount: Optional[float] = None to TradingSignal in strategies/base.py, but SignalContract in engine/contracts.py (unchanged by this PR) lacks a corresponding amount field. SignalContract.from_trading_signal calls cls(**signal.to_dict()) (contracts.py:32), and to_dict() uses dataclasses.asdict which now includes amount in the returned dict. Since SignalContract is a @dataclass(frozen=True) without an amount field, this raises TypeError: __init__() got an unexpected keyword argument 'amount' at runtime.
This method is called on every pipeline tick via ContractValidator.signal(signal) at signal_pipeline.py:215. The exception is caught by the try/except at signal_pipeline.py:216, converting every signal into a HOLD with confidence 0.0. The net effect is that no trade will ever execute through the SignalPipeline.
Prompt for agents
The new `amount` field added to TradingSignal in engine/strategies/base.py:54 must also be added to SignalContract in engine/contracts.py. The SignalContract dataclass (contracts.py:17-28) mirrors TradingSignal's fields and its from_trading_signal classmethod (contracts.py:31-32) does cls(**signal.to_dict()), which will now include the amount key. Add `amount: Optional[float] = None` to SignalContract between take_profit and reasoning, matching the field order in TradingSignal.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
✅ Created PR with unit tests: https://github.com/Moeabdelaziz007/AlphaAxiom/pull/62 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
money-machine/src-python/engine/strategies/base.py (1)
54-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
TradingSignal.amountin the canonical model.
amountis now part of the shared signal contract, but it is not validated in__post_init__. This allows invalid sizing values to propagate (e.g., negative or oversized fractions), which can lead to unsafe order sizing downstream.Proposed fix
`@dataclass` class TradingSignal: @@ amount: Optional[float] = None @@ def __post_init__(self) -> None: @@ if not 0.0 <= self.confidence <= 1.0: raise ValueError( f"confidence must be in [0.0, 1.0], got {self.confidence!r}" ) + if self.amount is not None: + try: + self.amount = float(self.amount) + except (TypeError, ValueError): + raise ValueError(f"amount must be numeric or None, got {self.amount!r}") + if not 0.0 <= self.amount <= 0.02: + raise ValueError( + f"amount must be in [0.0, 0.02] as portfolio fraction, got {self.amount!r}" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@money-machine/src-python/engine/strategies/base.py` around lines 54 - 71, The TradingSignal model's amount field is not validated in __post_init__, allowing negative or >1.0 sizing to slip through; update the __post_init__ in base.py to validate self.amount (if not None) is a number and within [0.0, 1.0], raising ValueError with a clear message if it isn't; keep this check alongside the existing action and confidence validations so INVALID_ACTIONS/VALID_ACTIONS and confidence logic remain unchanged.money-machine/src/lib/tauri.ts (1)
39-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd timeout and HTTP status handling to the IPC fetch fallback.
The development fallback in
sendIPCCommandlacks an AbortController timeout and HTTP status validation. This can cause indefinite hangs and opaque parse errors on non-2xx responses. The Python backend enforces a 5-second read timeout; the fetch fallback should match this hardening. Add an AbortController timeout and validateresponse.okbefore callingresponse.json().Suggested patch
async function sendIPCCommand<T>(command: string, payload: Record<string, unknown> = {}): Promise<T> { if (isTauri) { return await invoke(command.toLowerCase(), payload); } // Development fallback: direct API route + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10_000); const response = await fetch('/api/ipc', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ command, payload }), + signal: controller.signal, + }).finally(() => clearTimeout(timeout)); + + if (!response.ok) { + throw new Error(`IPC request failed: ${response.status} ${response.statusText}`); + } const data = await response.json();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@money-machine/src/lib/tauri.ts` around lines 39 - 49, The development fallback in sendIPCCommand lacks a timeout and HTTP status checks; update the fetch branch inside sendIPCCommand to use an AbortController with a 5_000 ms timeout (match the Python backend), attach controller.signal to fetch, and clear the timer after fetch completes; then check response.ok and throw a descriptive error (including status and body/text) if not ok before calling response.json() and returning the parsed value. Ensure the AbortController is aborted on timeout to prevent hangs and that any thrown error surfaces the HTTP status and body to aid debugging.
🧹 Nitpick comments (4)
money-machine/src-python/utils/ipc_server.py (1)
166-167: 💤 Low valueConsider narrowing the exception catch to network-related errors.
The broad
except Exceptionis flagged by static analysis (BLE001). While acceptable as a defensive catch-all for untrusted I/O, narrowing to(OSError, ConnectionError)would be more explicit about intent and avoid accidentally masking unrelated exceptions.♻️ Suggested refinement
- except Exception as exc: - return b"", {"error": f"Failed to read {label}: {exc}", "code": 400} + except (OSError, ConnectionError) as exc: + return b"", {"error": f"Failed to read {label}: {exc}", "code": 400}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@money-machine/src-python/utils/ipc_server.py` around lines 166 - 167, The broad except Exception in the IPC read handler should be narrowed to network/IO-related errors to avoid masking unrelated exceptions; change the except block that currently returns b"", {"error": f"Failed to read {label}: {exc}", "code": 400} to except (OSError, ConnectionError) as exc and leave the return value and error message intact so the handler still returns b"" and the same error dict; locate this change in the IPC server read routine where label is used (the try/except around the read/receive logic).money-machine/src-python/utils/config.py (1)
41-46: ⚡ Quick winNarrow exception handling for config file operations.
Catching
Exceptionhere can hide non-I/O coding defects and make failures harder to triage. Prefer handling expected config read/write errors explicitly.🔧 Suggested tightening
- except Exception as e: + except (OSError, json.JSONDecodeError, TypeError) as e: print(f"Warning: Could not load config file: {e}") ... - except Exception as e: + except (OSError, TypeError) as e: print(f"Error saving config: {e}") return FalseAlso applies to: 77-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@money-machine/src-python/utils/config.py` around lines 41 - 46, The current broad except Exception around opening and json.load hides unrelated bugs; narrow it to handle expected I/O/parsing errors only by catching FileNotFoundError, PermissionError/OSError and json.JSONDecodeError around the with open(config_path...) / json.load call (and similarly in the other block at 77-83), log a clear warning via print/processLogger that includes the exception, and re-raise or let other exceptions propagate instead of swallowing them; refer to the open(config_path, 'r') context, json.load, _without_secrets and _deep_merge to locate where to apply the specific except clauses.money-machine/public/_headers (1)
2-2: ⚡ Quick winExplicitly disable object/embed sources in CSP.
default-src 'self'still permits plugin/object content from same-origin. Addobject-src 'none'to tighten policy.🔐 Proposed fix
- Content-Security-Policy: default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:; connect-src 'self' https://oracle.axiomid.app https://aitrading.axiomid.app; frame-ancestors 'none'; base-uri 'self'; form-action 'self' + Content-Security-Policy: default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:; connect-src 'self' https://oracle.axiomid.app https://aitrading.axiomid.app; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@money-machine/public/_headers` at line 2, The CSP header currently allows same-origin plugin/object content via default-src; update the header line that defines Content-Security-Policy to explicitly disable plugin/embed sources by adding object-src 'none' (i.e., include the object-src 'none' directive alongside existing directives such as script-src, style-src, img-src, connect-src, etc.) so that object/embed content is blocked even from the same origin.money-machine/src/app/page.tsx (1)
20-20: ⚡ Quick winSpecify
sizeson the fill logo image.
next/imagewithfillshould includesizes; otherwise optimization can over-fetch larger image variants than needed. This container is fixed (h-16 w-16), so a fixed hint is appropriate.♻️ Proposed fix
- <Image src="/images/logo.png" alt="Money Machine" fill className="object-cover" priority /> + <Image + src="/images/logo.png" + alt="Money Machine" + fill + sizes="64px" + className="object-cover" + priority + />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@money-machine/src/app/page.tsx` at line 20, The Image element using next/image with fill (the Image component rendering "/images/logo.png" with fill and className "object-cover" in the page.tsx) lacks a sizes attribute, which can cause next/image to fetch larger variants than needed; add a sizes prop that matches the fixed container (h-16 w-16 → 64px) such as sizes="64px" (or an equivalent responsive media-query string) to ensure the loader requests the correct image size.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@money-machine/next-env.d.ts`:
- Line 3: Remove the brittle explicit import of "./.next/types/routes.d.ts" from
next-env.d.ts: delete the import line so the file remains framework-managed and
relies on the tsconfig.json include globs (e.g., .next/types/**/*.ts) to pick up
generated route types instead of importing them directly.
In `@money-machine/src-python/engine/trading_core.py`:
- Around line 10-14: CONFIG_LIMITS currently marks "initial_balance"
runtime-updatable but update_config() only mutates self.config and doesn't
reconcile portfolio state, causing desync between self.config["initial_balance"]
and self.portfolio.balance; remove "initial_balance" from runtime-updatable
fields in CONFIG_LIMITS (or mark it as startup-only) and ensure update_config()
does not accept or apply changes to initial_balance, leaving portfolio
initialization logic (where portfolio.balance is set) as the single source of
truth; update references to CONFIG_LIMITS and the update_config() method to
enforce this change.
In `@money-machine/src/components/StatusWidget.tsx`:
- Around line 57-60: The StatusWidget currently hardcodes the Oracle host string
in the JSX block; update StatusWidget to read the runtime-configured value
(e.g., from props, context, or a validated config util used by the app) instead
of the literal "oracle.axiomid.app" and render a safe fallback like
"unavailable" when the config is missing or invalid; locate the span showing the
host inside StatusWidget and replace the hardcoded text with the runtime value
(validate/normalize it first) so the UI reflects actual deployment
configuration.
---
Outside diff comments:
In `@money-machine/src-python/engine/strategies/base.py`:
- Around line 54-71: The TradingSignal model's amount field is not validated in
__post_init__, allowing negative or >1.0 sizing to slip through; update the
__post_init__ in base.py to validate self.amount (if not None) is a number and
within [0.0, 1.0], raising ValueError with a clear message if it isn't; keep
this check alongside the existing action and confidence validations so
INVALID_ACTIONS/VALID_ACTIONS and confidence logic remain unchanged.
In `@money-machine/src/lib/tauri.ts`:
- Around line 39-49: The development fallback in sendIPCCommand lacks a timeout
and HTTP status checks; update the fetch branch inside sendIPCCommand to use an
AbortController with a 5_000 ms timeout (match the Python backend), attach
controller.signal to fetch, and clear the timer after fetch completes; then
check response.ok and throw a descriptive error (including status and body/text)
if not ok before calling response.json() and returning the parsed value. Ensure
the AbortController is aborted on timeout to prevent hangs and that any thrown
error surfaces the HTTP status and body to aid debugging.
---
Nitpick comments:
In `@money-machine/public/_headers`:
- Line 2: The CSP header currently allows same-origin plugin/object content via
default-src; update the header line that defines Content-Security-Policy to
explicitly disable plugin/embed sources by adding object-src 'none' (i.e.,
include the object-src 'none' directive alongside existing directives such as
script-src, style-src, img-src, connect-src, etc.) so that object/embed content
is blocked even from the same origin.
In `@money-machine/src-python/utils/config.py`:
- Around line 41-46: The current broad except Exception around opening and
json.load hides unrelated bugs; narrow it to handle expected I/O/parsing errors
only by catching FileNotFoundError, PermissionError/OSError and
json.JSONDecodeError around the with open(config_path...) / json.load call (and
similarly in the other block at 77-83), log a clear warning via
print/processLogger that includes the exception, and re-raise or let other
exceptions propagate instead of swallowing them; refer to the open(config_path,
'r') context, json.load, _without_secrets and _deep_merge to locate where to
apply the specific except clauses.
In `@money-machine/src-python/utils/ipc_server.py`:
- Around line 166-167: The broad except Exception in the IPC read handler should
be narrowed to network/IO-related errors to avoid masking unrelated exceptions;
change the except block that currently returns b"", {"error": f"Failed to read
{label}: {exc}", "code": 400} to except (OSError, ConnectionError) as exc and
leave the return value and error message intact so the handler still returns b""
and the same error dict; locate this change in the IPC server read routine where
label is used (the try/except around the read/receive logic).
In `@money-machine/src/app/page.tsx`:
- Line 20: The Image element using next/image with fill (the Image component
rendering "/images/logo.png" with fill and className "object-cover" in the
page.tsx) lacks a sizes attribute, which can cause next/image to fetch larger
variants than needed; add a sizes prop that matches the fixed container (h-16
w-16 → 64px) such as sizes="64px" (or an equivalent responsive media-query
string) to ensure the loader requests the correct image size.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c9ec9ad6-f347-4926-aab8-2e0799933a63
⛔ Files ignored due to path filters (7)
money-machine/package-lock.jsonis excluded by!**/package-lock.jsonmoney-machine/public/images/logo.pngis excluded by!**/*.pngmoney-machine/src-tauri/Cargo.lockis excluded by!**/*.lockmoney-machine/src-tauri/icons/128x128.pngis excluded by!**/*.pngmoney-machine/src-tauri/icons/128x128@2x.pngis excluded by!**/*.pngmoney-machine/src-tauri/icons/32x32.pngis excluded by!**/*.pngmoney-machine/src-tauri/icons/icon.pngis excluded by!**/*.png
📒 Files selected for processing (27)
README.mdmoney-machine/.gitignoremoney-machine/eslint.config.mjsmoney-machine/next-env.d.tsmoney-machine/package.jsonmoney-machine/public/_headersmoney-machine/src-python/engine/signal_generator.pymoney-machine/src-python/engine/strategies/base.pymoney-machine/src-python/engine/trading_core.pymoney-machine/src-python/tests/test_config_security.pymoney-machine/src-python/tests/test_ipc_auth.pymoney-machine/src-python/utils/config.pymoney-machine/src-python/utils/ipc_server.pymoney-machine/src-tauri/Cargo.tomlmoney-machine/src-tauri/src/lib.rsmoney-machine/src-tauri/tauri.conf.jsonmoney-machine/src/app/globals.cssmoney-machine/src/app/layout.tsxmoney-machine/src/app/page.tsxmoney-machine/src/components/ControlPanel.tsxmoney-machine/src/components/PnLWidget.tsxmoney-machine/src/components/StatusWidget.tsxmoney-machine/src/components/TradesTable.tsxmoney-machine/src/lib/tauri.tsmoney-machine/src/store/useAppStore.tsmoney-machine/tsconfig.jsonvercel.json
| @@ -0,0 +1,6 @@ | |||
| /// <reference types="next" /> | |||
| /// <reference types="next/image-types/global" /> | |||
| import "./.next/types/routes.d.ts"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether next-env.d.ts imports generated artifacts directly.
rg -n '^\s*import\s+["'\'']\./\.next/types/routes\.d\.ts["'\''];?\s*$' money-machine/next-env.d.ts
# Check if generated file exists in repo checkout (clean CI often won't have it).
test -f money-machine/.next/types/routes.d.ts; echo "routes.d.ts exists? exit=$?"
# Inspect tsconfig include entries that already cover generated types.
rg -n '"\.next/types/\*\*/\*\.ts"|"\.next/dev/types/\*\*/\*\.ts"' money-machine/tsconfig.jsonRepository: Moeabdelaziz007/AlphaAxiom
Length of output: 198
Remove direct import of generated .next route types from next-env.d.ts.
The import of ./.next/types/routes.d.ts is brittle and breaks type-checking in clean environments before the .next directory is generated. This is already covered by tsconfig.json include globs (.next/types/**/*.ts), making the explicit import redundant. Remove the import and keep next-env.d.ts framework-managed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@money-machine/next-env.d.ts` at line 3, Remove the brittle explicit import of
"./.next/types/routes.d.ts" from next-env.d.ts: delete the import line so the
file remains framework-managed and relies on the tsconfig.json include globs
(e.g., .next/types/**/*.ts) to pick up generated route types instead of
importing them directly.
| CONFIG_LIMITS = { | ||
| "initial_balance": (100.0, 1_000_000.0, False), | ||
| "max_risk_per_trade": (0.0, 0.1, True), | ||
| "max_daily_loss": (0.0, 0.2, True), | ||
| } |
There was a problem hiding this comment.
Runtime initial_balance updates can desynchronize engine state.
initial_balance is whitelisted for runtime updates, but update_config() only updates self.config; it does not reconcile self.portfolio.balance / portfolio-derived state. A successful runtime update can therefore leave config and live portfolio state inconsistent.
💡 Suggested fix (disallow runtime mutation for startup-only field)
CONFIG_LIMITS = {
- "initial_balance": (100.0, 1_000_000.0, False),
"max_risk_per_trade": (0.0, 0.1, True),
"max_daily_loss": (0.0, 0.2, True),
}Also applies to: 188-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@money-machine/src-python/engine/trading_core.py` around lines 10 - 14,
CONFIG_LIMITS currently marks "initial_balance" runtime-updatable but
update_config() only mutates self.config and doesn't reconcile portfolio state,
causing desync between self.config["initial_balance"] and
self.portfolio.balance; remove "initial_balance" from runtime-updatable fields
in CONFIG_LIMITS (or mark it as startup-only) and ensure update_config() does
not accept or apply changes to initial_balance, leaving portfolio initialization
logic (where portfolio.balance is set) as the single source of truth; update
references to CONFIG_LIMITS and the update_config() method to enforce this
change.
| <div className="mb-3 flex items-center justify-between rounded-xl border border-white/10 bg-white/[0.03] px-3 py-2"> | ||
| <span className="text-[var(--text-muted)]">Oracle</span> | ||
| <span className="text-[var(--accent-blue)]">oracle.axiomid.app</span> | ||
| </div> |
There was a problem hiding this comment.
Avoid hardcoding the Oracle host in status UI.
This value can drift from runtime config/deployment state and present stale status information. Prefer sourcing it from validated runtime config (or showing “unavailable” when not configured).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@money-machine/src/components/StatusWidget.tsx` around lines 57 - 60, The
StatusWidget currently hardcodes the Oracle host string in the JSX block; update
StatusWidget to read the runtime-configured value (e.g., from props, context, or
a validated config util used by the app) instead of the literal
"oracle.axiomid.app" and render a safe fallback like "unavailable" when the
config is missing or invalid; locate the span showing the host inside
StatusWidget and replace the hardcoded text with the runtime value
(validate/normalize it first) so the UI reflects actual deployment
configuration.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
money-machine/src-python/engine/signal_generator.py (1)
193-195: 💤 Low valueConsider defaulting
amount_pctto0.0instead of potentiallyNone.When
data.get("amount_pct")returnsNone, the metadata will contain{"amount_pct": None}. Whilemain.pyhandles this withor 0.0, it would be more explicit to default here:- metadata={"amount_pct": data.get("amount_pct")}, + metadata={"amount_pct": data.get("amount_pct", 0.0)},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@money-machine/src-python/engine/signal_generator.py` around lines 193 - 195, The metadata currently sets "amount_pct" to data.get("amount_pct") which can be None; change it to default to 0.0 (e.g., use data.get("amount_pct", 0.0) or coerce with float(...) if needed) so the metadata contains a numeric value instead of None; update the metadata assignment in the Signal creation (the block that sets reasoning=... and metadata={"amount_pct": ...}) to use the 0.0 default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@money-machine/.github/workflows/build.yml`:
- Line 115: Replace every floating GitHub Action version tag in the workflow
with the corresponding full commit SHA and add a comment with the human-readable
version next to each use: update uses for actions/checkout@v4,
dtolnay/rust-action@stable, actions/setup-node@v4, actions/setup-python@v5,
tauri-apps/tauri-action@v0, actions/upload-artifact@v4,
actions/download-artifact@v4, and softprops/action-gh-release@v2 to their
immutable 40-character commit SHAs (and append a comment like "# v2.0.0" for
readability); ensure you change the uses strings in the workflow to the SHAs and
leave comments showing the original tag versions so Dependabot/maintainers can
track updates.
---
Nitpick comments:
In `@money-machine/src-python/engine/signal_generator.py`:
- Around line 193-195: The metadata currently sets "amount_pct" to
data.get("amount_pct") which can be None; change it to default to 0.0 (e.g., use
data.get("amount_pct", 0.0) or coerce with float(...) if needed) so the metadata
contains a numeric value instead of None; update the metadata assignment in the
Signal creation (the block that sets reasoning=... and metadata={"amount_pct":
...}) to use the 0.0 default.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e0f6bb04-0773-488f-9c7a-e6dcad1bd4ec
⛔ Files ignored due to path filters (2)
money-machine/package-lock.jsonis excluded by!**/package-lock.jsonmoney-machine/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/release.ymlCHANGELOG.mdmoney-machine/.github/workflows/build.ymlmoney-machine/package.jsonmoney-machine/src-python/engine/adapters/mt5.pymoney-machine/src-python/engine/signal_generator.pymoney-machine/src-python/main.pymoney-machine/src-tauri/Cargo.tomlmoney-machine/src-tauri/tauri.conf.jsonmoney-machine/tsconfig.json
💤 Files with no reviewable changes (1)
- money-machine/src-python/engine/adapters/mt5.py
✅ Files skipped from review due to trivial changes (2)
- CHANGELOG.md
- .github/release.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- money-machine/src-tauri/Cargo.toml
- money-machine/src-tauri/tauri.conf.json
- money-machine/package.json
- money-machine/tsconfig.json
|
|
||
| - name: Create Release | ||
| uses: softprops/action-gh-release@v1 | ||
| uses: softprops/action-gh-release@v2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify workflow actions that are not pinned to a full commit SHA.
# Expected: any remaining lines are candidates to pin.
rg -nP '^\s*uses:\s*[^@\s]+@[^\s]+$' -- '**/*.yml' '**/*.yaml' \
| rg -nPv '@[0-9a-fA-F]{40}$'Repository: Moeabdelaziz007/AlphaAxiom
Length of output: 169
🏁 Script executed:
#!/bin/bash
# Find the workflow file
find . -type f -path "*money-machine/.github/workflows/build.yml" 2>/dev/nullRepository: Moeabdelaziz007/AlphaAxiom
Length of output: 114
🏁 Script executed:
#!/bin/bash
# If file exists, show content around line 115
if [ -f "./money-machine/.github/workflows/build.yml" ]; then
echo "=== File found. Lines 110-120 ===";
sed -n '110,120p' "./money-machine/.github/workflows/build.yml" | cat -n;
else
echo "File not found at expected path. Searching for build.yml files:";
find . -type f -name "build.yml" 2>/dev/null;
fiRepository: Moeabdelaziz007/AlphaAxiom
Length of output: 537
🏁 Script executed:
#!/bin/bash
# Search for any uses of softprops/action-gh-release in the repo
rg 'softprops/action-gh-release' -nRepository: Moeabdelaziz007/AlphaAxiom
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Show all 'uses:' lines in the build.yml file
cat -n "./money-machine/.github/workflows/build.yml" | grep 'uses:'Repository: Moeabdelaziz007/AlphaAxiom
Length of output: 445
🌐 Web query:
GitHub Actions security best practices version pinning commit SHA
💡 Result:
GitHub Actions security best practices strongly recommend pinning actions to full-length commit SHAs to prevent supply chain attacks where malicious code could be injected via mutable tags or compromised repositories [1][2][3]. This is the only way to use actions as immutable releases, as tags can be rewritten or deleted even by trusted maintainers [1][4]. Official GitHub documentation states: "Pinning an action to a full-length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload" [1]. Example workflow change: uses: actions/checkout@v4 becomes: uses: actions/checkout@692973e # v4.1.0 [5][6] To find the SHA for a tag: - git ls-remote --tags https://github.com/owner/repo.git refs/tags/v4 [7] - Visit the repo, switch to the tag, copy the full SHA [6] - Check workflow run logs under "Setup Job" [6] Add a comment with the version for readability, e.g., # v4.1.0 [3][7]. Use Dependabot to automate updates: it creates PRs updating the SHA and version comment when new releases occur [3][8][9]. Tools like pin-github-action or ensure-sha-pinned-actions can help enforce/automate pinning [10][11][12]. Organizations can enforce SHA pinning via policies at repo/org/enterprise levels; workflows using tags will fail [2][3]. The policy checks the full dependency tree, including sub-actions [3]. Upcoming 2026 features include workflow dependencies lockfiles for transitive pinning and execution policies [13]. Additional practices: - Verify SHA is from the official repo, not a fork [1] - Prefer verified creators on Marketplace [1] - Use read-only GITHUB_TOKEN by default, escalate per-job [14] - Avoid pull_request_target for untrusted code [15] - Implement cooldowns (7-14 days) before updating [16]
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 3: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 4: https://dev.to/kanywst/hacking-github-from-tag-rewrites-to-dangling-commits-where-the-git-protocol-trusts-you-without-2o4h
- 5: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 6: https://stackoverflow.com/questions/78903499/how-do-i-pin-an-action-to-a-specific-sha
- 7: Pin all GitHub Actions to immutable SHA commits for supply chain security github/gh-aw#5691
- 8: github actions update fails when action is pinned to commit hash dependabot/dependabot-core#11253
- 9: For actions that are pinned-by-hash, bump the human readable version number in the code comment dependabot/dependabot-core#4691
- 10: https://www.npmjs.com/package/pin-github-action
- 11: https://github.com/marketplace/actions/ensure-sha-pinned-actions
- 12: https://github.com/mheap/pin-github-action
- 13: https://github.blog/news-insights/product-news/whats-coming-to-our-github-actions-2026-security-roadmap
- 14: https://docs.github.com/actions/learn-github-actions/security-hardening-for-github-actions
- 15: https://secure-pipelines.com/ci-cd-security/github-actions-security-definitive-guide/
- 16: https://www.wiz.io/blog/github-actions-security-guide
Pin all GitHub Actions to immutable commit SHAs.
All eight actions in this workflow use floating version tags (v4, v5, v0, v2, stable), which can change upstream and weaken CI/CD supply-chain guarantees. GitHub's official security guidance recommends pinning to full 40-character commit SHAs as the only way to ensure immutability. Apply this across the entire file:
- Line 38:
actions/checkout@v4 - Line 41:
dtolnay/rust-action@stable - Line 52:
actions/setup-node@v4 - Line 65:
actions/setup-python@v5 - Line 75:
tauri-apps/tauri-action@v0 - Line 90:
actions/upload-artifact@v4 - Line 110:
actions/download-artifact@v4 - Line 115:
softprops/action-gh-release@v2
Include a comment with the version for readability (e.g., # v2.0.0). Use Dependabot to automate updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@money-machine/.github/workflows/build.yml` at line 115, Replace every
floating GitHub Action version tag in the workflow with the corresponding full
commit SHA and add a comment with the human-readable version next to each use:
update uses for actions/checkout@v4, dtolnay/rust-action@stable,
actions/setup-node@v4, actions/setup-python@v5, tauri-apps/tauri-action@v0,
actions/upload-artifact@v4, actions/download-artifact@v4, and
softprops/action-gh-release@v2 to their immutable 40-character commit SHAs (and
append a comment like "# v2.0.0" for readability); ensure you change the uses
strings in the workflow to the SHAs and leave comments showing the original tag
versions so Dependabot/maintainers can track updates.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Docstrings generation - SUCCESS |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
Docstrings generation was requested by @Moeabdelaziz007. * https://github.com/Moeabdelaziz007/AlphaAxiom/pull/61#issuecomment-4447757058 The following files were modified: * `money-machine/src-python/engine/adapters/mt5.py` * `money-machine/src-python/engine/signal_generator.py` * `money-machine/src-python/engine/trading_core.py` * `money-machine/src-python/main.py` * `money-machine/src-python/tests/test_config_security.py` * `money-machine/src-python/tests/test_ipc_auth.py` * `money-machine/src-python/utils/config.py` * `money-machine/src-python/utils/ipc_server.py` * `money-machine/src-tauri/src/lib.rs` * `money-machine/src/app/layout.tsx` * `money-machine/src/app/page.tsx` * `money-machine/src/components/ControlPanel.tsx` * `money-machine/src/components/PnLWidget.tsx` * `money-machine/src/components/StatusWidget.tsx` * `money-machine/src/components/TradesTable.tsx` * `money-machine/src/lib/tauri.ts`
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
No files have been changed in this PR. Unable to generate unit tests. |
|
✅ Created PR with unit tests: https://github.com/Moeabdelaziz007/AlphaAxiom/pull/64 |
Docstrings generation was requested by @Moeabdelaziz007. * https://github.com/Moeabdelaziz007/AlphaAxiom/pull/61#issuecomment-4447757058 The following files were modified: * `money-machine/src-python/engine/adapters/mt5.py` * `money-machine/src-python/engine/signal_generator.py` * `money-machine/src-python/engine/trading_core.py` * `money-machine/src-python/main.py` * `money-machine/src-python/tests/test_config_security.py` * `money-machine/src-python/tests/test_ipc_auth.py` * `money-machine/src-python/utils/config.py` * `money-machine/src-python/utils/ipc_server.py` * `money-machine/src-tauri/src/lib.rs` * `money-machine/src/app/layout.tsx` * `money-machine/src/app/page.tsx` * `money-machine/src/components/ControlPanel.tsx` * `money-machine/src/components/PnLWidget.tsx` * `money-machine/src/components/StatusWidget.tsx` * `money-machine/src/components/TradesTable.tsx` * `money-machine/src/lib/tauri.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
✅ Created PR with unit tests: https://github.com/Moeabdelaziz007/AlphaAxiom/pull/65 |
1 participant
Summary
config.json, IPC requests now enforce size limits and read timeouts,TradingSignalnow has one canonical dataclass, and runtime config updates are whitelist/range validated.tsconfig, and removed fake demo trades from the UI._headers, CSP/security headers, and dashboard domain updates toaitrading.axiomid.app.postcssto a patched version.Review & Testing Checklist for Human
money-machine/src-python/, especially the secret redaction, IPC limits/timeouts, and config whitelist/ranges before any live trading use.aitrading.axiomid.appremains the intended dashboard domain.Recommended end-to-end test plan: start the Python sidecar, launch
npm run tauri dev, confirm the dashboard connects, test Start/Pause Shadow Trading, Ghost/Floating/Pinned controls, then send invalid/oversized IPC/config payloads and confirm they are rejected.Notes
Local verification completed:
npm --prefix money-machine run lintcd money-machine && npx tsc --noEmit --pretty falsenpm --prefix money-machine run buildcargo +stable test --manifest-path money-machine/src-tauri/Cargo.tomlpython -m pytest money-machine/src-python/tests/test_config_security.py money-machine/src-python/tests/test_ipc_auth.py money-machine/src-python/tests/test_strategies.py money-machine/src-python/tests/test_risk_shield.py -q→ 108 passednpm --prefix money-machine audit --omit=dev --json→ 0 vulnerabilitiesCloudflare/Vercel check:
aitrading.axiomid.appreturns 200 on Vercel;oracle.axiomid.appcurrently returns VercelDEPLOYMENT_NOT_FOUND404, so the Oracle/Worker deployment still needs project-side configuration outside this PR.Link to Devin session: https://app.devin.ai/sessions/e7508e5e3b974b8793307bda88eed3e8
Requested by: @Moeabdelaziz007
Summary by CodeRabbit
Release Notes
New Features
Security
Chores