Skip to content
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

Add command for redacting backups #318

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

josephschorr
Copy link
Member

@josephschorr josephschorr commented Dec 28, 2023

Helpful for providing community support without leaking the names, comments and/or object IDs in the permission system

Related: #194

Copy link
Contributor

@vroldanbet vroldanbet left a comment

Choose a reason for hiding this comment

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

LGTM, just a comment

Can we 🙏🏻 merge #316 first? It's had its fair share of rebasing already

Relations map[string]string

// ObjectIDs is the map of original object IDs to their redacted names.
ObjectIDs map[string]string
Copy link
Contributor

Choose a reason for hiding this comment

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

For very large large backupfiles, this may not fit in memory and cause the process to crash. A hash of ObjectIDs could be used, trading off memory for CPU usage, which may make the redaction take longer for large files but at least it wouldn't crash.

Copy link
Member Author

Choose a reason for hiding this comment

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

My concern with using a hash is the risk of conflicts

caveatDef.Name = redactionMap.Caveats[caveatDef.Name]
}

// TODO: Redact caveat parameters.
Copy link
Contributor

Choose a reason for hiding this comment

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

zed should show a warning when caveats are used, or fail, otherwise users may falsely assume everything will be redacted, including caveat args.

I lean towards alerting so the user makes their call about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha

Helpful for providing community support without leaking the names, comments and/or object IDs in the permission system
@josephschorr
Copy link
Member Author

Rebased

@vroldanbet vroldanbet added this pull request to the merge queue Jan 3, 2024
Merged via the queue into authzed:main with commit 56c0175 Jan 3, 2024
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants