fix: security hardening and non-atomic move bugfix#6
Open
TheTrustedAdvisor wants to merge 1 commit intoLars147:mainfrom
Open
fix: security hardening and non-atomic move bugfix#6TheTrustedAdvisor wants to merge 1 commit intoLars147:mainfrom
TheTrustedAdvisor wants to merge 1 commit intoLars147:mainfrom
Conversation
- Reverse move_recipe_in_plan operation order to prevent data loss (add first, then remove — duplicate is safer than lost recipe) - Warn about --password CLI flag security risk, support COOKIDOO_PASSWORD env var - Replace 7 bare except: clauses with specific exception types - Add cookidoo_categories.json to .gitignore - Fix hardcoded URLs bypassing COOKIDOO_BASE/LOCALE constants Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the CLI’s security posture and improves resilience by addressing a non-atomic “move recipe” workflow, avoiding insecure password handling patterns, tightening exception handling, and removing locale-specific hardcoded URLs.
Changes:
- Make
move_recipe_in_planadd-first-then-remove to avoid data loss on partial failure. - Prefer
COOKIDOO_PASSWORDenv var for login and mark--passwordas insecure in help text. - Replace several bare
except:clauses with explicit exception types; fix hardcoded shopping URLs; ignore categories cache file in git.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tmx_cli.py | Updates move semantics, login password sourcing/help text, exception specificity, and locale-aware shopping URLs. |
| .gitignore | Adds cookidoo_categories.json to prevent committing generated cache data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Only remove from old date after successful add | ||
| success, msg = remove_recipe_from_plan(recipe_id, from_date) | ||
| if not success: | ||
| return False, f"Verschoben, aber Entfernen vom alten Datum fehlgeschlagen: {msg}" |
Comment on lines
657
to
665
| if SEARCH_TOKEN_FILE.exists(): | ||
| try: | ||
| with open(SEARCH_TOKEN_FILE, "r") as f: | ||
| cached = json.load(f) | ||
| # Check if still valid (with 5 min buffer) | ||
| if cached.get("validUntil", 0) > dt.datetime.now().timestamp() + 300: | ||
| return cached.get("apiKey") | ||
| except: | ||
| except (json.JSONDecodeError, KeyError, OSError): | ||
| pass |
Comment on lines
1886
to
1892
| try: | ||
| with open(CATEGORIES_CACHE_FILE, "r", encoding="utf-8") as f: | ||
| cache_data = json.load(f) | ||
| ts = cache_data.get("timestamp", "")[:16].replace("T", " ") | ||
| print(f" (aus Cache, Stand: {ts} UTC)") | ||
| except: | ||
| except (json.JSONDecodeError, OSError): | ||
| print(" (aus Cache)") |
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.
Summary
move_recipe_in_plannow adds to the new date before removing from the old date — a duplicate is recoverable, a lost recipe is not.--passwordCLI flag is now marked as insecure in help text;COOKIDOO_PASSWORDenv var is checked before falling back to interactive prompt, keeping credentials out ofpsoutput.except:clauses replaced with specific exception types (Exception,json.JSONDecodeError,KeyError,OSError,ValueError,TypeError) for better error visibility and debuggability.cookidoo_categories.jsonadded to the Cache section of.gitignoreto prevent accidental commit.https://cookidoo.de/shopping/de-DEhardcoded URLs inclear_shopping_listandadd_custom_item_to_shopping_listreplaced withCOOKIDOO_BASE/LOCALEconstants so locale changes apply everywhere.Test plan
tmx loginwithout--passwordflag prompts interactively as beforeCOOKIDOO_PASSWORD=secret tmx login --email user@example.comlogs in without prompttmx login --helpshows the(UNSICHER - nutze COOKIDOO_PASSWORD Umgebungsvariable)warning on--passwordtmx plan move <recipe_id> <from_date> <to_date>succeeds without data loss (recipe appears on new date, removed from old)move_recipe_in_plan: recipe should remain on original date (not lost)cookidoo_categories.jsonis not tracked by git aftergit add .de-DElocale ifLOCALEconstant is changedexcept:lines remain:grep -n "except:" tmx_cli.pyreturns empty🤖 Generated with Claude Code