Skip to content

expand firebase storage rules coverage - #1002

Merged
0xallam merged 8 commits into
mainfrom
devin/1786057038-firebase-storage-rules-coverage
Aug 7, 2026
Merged

expand firebase storage rules coverage#1002
0xallam merged 8 commits into
mainfrom
devin/1786057038-firebase-storage-rules-coverage

Conversation

@bearsyankees

@bearsyankees bearsyankees commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Cloud Storage sits behind two front doors with different authorization engines, and the Firebase skill only documented one of them:

Front door Authorization engine
storage.googleapis.com/<bucket>/<object>, /storage/v1/b/<bucket> GCS IAM + per-object ACLs
firebasestorage.googleapis.com/v0/b/<bucket>/o Firebase Storage Security Rules

The string firebasestorage.googleapis.com appeared nowhere in the repo, so an agent that probed the GCS door, got 403 on everything, and saw no allUsers bucket grant would conclude the bucket was locked down — while rules like allow read, write: if request.time < timestamp.date(2027, 5, 4) (the console test-mode default) grant unauthenticated read and write to every object through the other door. The skill now documents both doors, the unauth/anonymous/low-privilege list+read+write probe matrix, the dangerous rule patterns (time gate, {allPaths=**}, request.auth != null as sole gate, claim-presence checks), Storage rules' OR-across-matches semantics, bucket discovery, and object-ACL persistence after a rules fix.

It also fixes a whitebox failure mode: absent rules IaC was being read as nothing to review. Missing storage/firestore blocks in firebase.json, or a referenced rules file absent from the tree, means the live rules are unmanaged and default to test-mode open — that is itself a finding and must force live probing.

Second, the coordinator couldn't see any of this when picking skills. get_available_skills() returned names only, so technologies/firebase_firestore was all the root agent had to go on and Storage coverage was invisible at selection time. The skill is renamed technologies/firebase and descriptions are now exposed and rendered:

-{% for category, names in available_skills | dictsort -%}
-- {{ category }}: {{ names | join(', ') }}
+{% for category, skills in available_skills | dictsort -%}
+{% for skill in skills -%}
+- {{ category }}/{{ skill.name }}{% if skill.description %}: {{ skill.description }}{% endif %}

Supporting cleanup in strix/skills/__init__.py: frontmatter is parsed once by a shared _parse_skill_content() -> (metadata, body) used by both discovery and load_skills() (replacing the body-only _FRONTMATTER_PATTERN.sub), via yaml.safe_load so block scalars and comments in a description can't leak a malformed value into every system prompt — unparseable or non-mapping frontmatter warns and yields empty metadata while the body still loads. Metadata reads are cached on (path, mtime_ns, size) so exposing descriptions doesn't add a file read per skill per prompt render, and _qualified_skill_file_for_name() returns Path | None instead of a single-element list so a missing file warns rather than being silently dropped. Skill resolution itself is still recomputed per call, so register_skill_dir() and precedence/shadowing behave as before.

Link to Devin session: https://app.devin.ai/sessions/ba80df57096b414cabe62ae4556b147f
Requested by: @bearsyankees

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR expands the Firebase skill to cover Firebase Storage Security Rules separately from GCS IAM and ACLs, while exposing skill descriptions in agent prompts.

  • Renames the Firebase/Firestore skill and documents Storage probing, authorization boundaries, rule patterns, and white-box review.
  • Parses skill frontmatter through PyYAML and caches metadata used by the available-skills catalog.
  • Adds regression coverage for multiline, blank-line, and commented YAML descriptions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
strix/skills/init.py Replaces the prior line-oriented metadata handling with YAML parsing, resolving all three previously reported multiline-description failures.
tests/test_skill_dir_extension.py Adds coverage for multiline descriptions, embedded blank lines, trailing block-scalar comments, malformed frontmatter, and prompt rendering.
strix/agents/prompts/system_prompt.jinja Renders each selectable skill with its qualified name and normalized optional description.
strix/skills/technologies/firebase.md Broadens Firebase testing guidance to distinguish Storage Rules access from GCS IAM and object ACL behavior.

Reviews (5): Last reviewed commit: "Parse skill frontmatter with YAML" | Re-trigger Greptile

Comment thread strix/skills/__init__.py Outdated
@bearsyankees

Copy link
Copy Markdown
Collaborator Author

@greptile

@devin-ai-integration

Copy link
Copy Markdown
Contributor

@greptile

Comment thread strix/skills/__init__.py Outdated
@devin-ai-integration

Copy link
Copy Markdown
Contributor

@greptile

Comment thread strix/skills/__init__.py Outdated
@devin-ai-integration

Copy link
Copy Markdown
Contributor

@greptile

@0xallam
0xallam merged commit 9dae766 into main Aug 7, 2026
1 check passed
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.

2 participants