Skip to content

Mask tenant script key in fetch logs and error messages - #15

Merged
ehs5 merged 2 commits into
masterfrom
devin/1790601174-mask-script-key
Sep 28, 2026
Merged

ehs5 merged 2 commits into
masterfrom
devin/1790601174-mask-script-key

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #14.

The key still goes over the wire unchanged (build_script_url is untouched, so requests.get gets the real URL) — only what we print is scrubbed. Two new helpers on FetchService:

mask_key(text, tenant) -> text.replace(tenant["key"], "***")   # no-op when key is empty
fail(error, tenant)    -> print(mask_key(error, tenant)); return None, error_masked

get_superoffice_data now logs mask_key(script_url, ...) and routes every error return through fail(), instead of print(error); return None, error. Masking the whole error string (not just the URL we build) also covers the requests exceptions, which embed the full request URL — e.g. ConnectionError: Max retries exceeded with url: ...&key=<secret>... leaked the key too, not just the invalid-JSON message the issue names.

Out of scope, flag if you want it changed: crmfetch show still prints the key in its details table, which looks deliberate.

Verification

  • python -m pytest tests -q → 87 passed. New tests cover: key absent from the invalid-JSON error, from a connection error containing the URL, and from --verbose stdout; plus a test that the real key is still sent to requests.get.
  • Clean venv, pip install ., crmfetch --help works and tkinter is not imported.
  • Live fetch against the SOD demo tenant with the installed CLI: 924 files created (Scripts 384, Screens 400, Triggers 98, Tables 35, ScreenChoosers 4, Scheduled tasks 3) — a healthy baseline, no parse errors. The --verbose endpoint log printed key=***, and grepping the whole CLI output for the real key value returned 0 hits.

Not verified against SOD: the error paths (invalid JSON, connection/SSL/HTTP failures) — the tenant responded normally, so those were only exercised against a local stub endpoint and in unit tests.

Link to Devin session: https://app.devin.ai/sessions/6daa0b67c92f45619f580de2a1e3c212
Open in Devin Desktop: https://app.devin.ai/desktop/session/6daa0b67c92f45619f580de2a1e3c212?variant=devin
Requested by: @ehs5

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

espen.steen and others added 2 commits September 28, 2026 13:29
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot force-pushed the devin/1790601174-mask-script-key branch from 798759b to 9f0587b Compare September 28, 2026 13:29
@ehs5
ehs5 merged commit 4520671 into master Sep 28, 2026
3 checks passed

@ehs5 ehs5 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged.

@ehs5
ehs5 deleted the devin/1790601174-mask-script-key branch September 28, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Script key is logged in plaintext in fetch URL

1 participant